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

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 118829
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail  like 118829
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 118829
 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-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt 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-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-arm64-arm64-libvirt-qcow2 12 migrate-support-checkfail never pass
 test-arm64-arm64-libvirt-qcow2 13 saverestore-support-checkfail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass

version targeted for testing:
 libvirt  554a5edcb46ff972fed45b851d70823b923fec6a
baseline version:
 libvirt  a90a1bf9e12bc23430f23033c671701ce4251cb6

Last test of basis   118829  2018-02-10 09:19:18 Z3 days
Testing same since   119049  2018-02-13 04:20:05 Z0 days1 attempts


People who touched revisions under test:
  Andrea Bolognani 
  Chen Hanxiao 
  Daniel P. Berrangé 
  John Ferlan 
  Michal Privoznik 
  Peter Krempa 

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



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

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

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

[Xen-devel] xl vcpu-set in HVM domU fails change cpu online count

[Olaf Hering]

xl vcpu-set with a HVM domU sends an ACPI event to do hot-add of cpus. But if 
the vcpu count is decreased the domU does not offline any cpu. So far the only 
way to enforce offlining is to process the output of 'xenstore-watch cpu'.

Why is HVM excluded in drivers/xen/cpu_hotplug.c:setup_vcpu_hotplug_event()? I 
think that would be the place to receive notifications from xenstore about cpus 
being offlined/onlined.

Olaf


pgp9yg8bDBrkc.pgp
Description: Digitale Signatur von OpenPGP
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [RFC PATCH 24/49] ARM: new VGIC: Add IRQ sync/flush framework

[Julien Grall]

Hi Andre,

On 09/02/18 14:39, 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 kvm_vgic_flush_hwstate and kvm_vgic_sync_hwstate, which


You probably want to update the names here.


gets called on guest entry and exit.
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 
---
  xen/arch/arm/vgic/vgic.c | 246 +++
  xen/arch/arm/vgic/vgic.h |   2 +
  2 files changed, 248 insertions(+)

diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c
index a4efd1fd03..a1f77130d4 100644
--- a/xen/arch/arm/vgic/vgic.c
+++ b/xen/arch/arm/vgic/vgic.c
@@ -380,6 +380,252 @@ int vgic_inject_irq(struct domain *d, struct vcpu *vcpu, 
unsigned int intid,
  return 0;
  }
  
+/**

+ * vgic_prune_ap_list - Remove non-relevant interrupts from the list
+ *
+ * @vcpu: The VCPU pointer
+ *
+ * Go over the list of "interesting" interrupts, and prune those that we
+ * won't have to consider in the near future.
+ */
+static void vgic_prune_ap_list(struct vcpu *vcpu)
+{
+struct vgic_cpu *vgic_cpu = >arch.vgic_cpu;
+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 )


See my comment on patch #22, this is where I am worry about going 
through the list every time we enter to the hypervisor from the guest.



+{
+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_cpu.ap_list_lock, flags);
+spin_lock(>arch.vgic_cpu.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_cpu;
+
+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_cpu.ap_list_lock);
+spin_unlock_irqrestore(>arch.vgic_cpu.ap_list_lock, flags);
+goto retry;
+}
+
+spin_unlock_irqrestore(_cpu->ap_list_lock, flags);
+}
+
+static inline void vgic_fold_lr_state(struct vcpu *vcpu)
+{
+}
+
+/* Requires the irq_lock to be held. */
+static inline void vgic_populate_lr(struct vcpu *vcpu,
+struct vgic_irq *irq, int lr)
+{
+ASSERT(spin_is_locked(>irq_lock));
+}
+
+static inline void vgic_clear_lr(struct vcpu *vcpu, int lr)
+{
+}
+
+static inline void vgic_set_underflow(struct vcpu *vcpu)
+{
+}
+
+/* Requires the ap_list_lock to be held. */
+static int compute_ap_list_depth(struct vcpu *vcpu)
+{
+struct vgic_cpu *vgic_cpu = >arch.vgic_cpu;
+struct vgic_irq *irq;
+int count = 0;
+
+ASSERT(spin_is_locked(_cpu->ap_list_lock));
+
+list_for_each_entry(irq, _cpu->ap_list_head, ap_list)


Here another example.


+{
+

Re: [Xen-devel] [PATCH v2] tools/libxl: mark special pages as reserved in e820 map for PVH

[Wei Liu]

On Tue, Feb 13, 2018 at 12:57:31PM +0100, Juergen Gross wrote:
> On 13/02/18 12:55, Wei Liu wrote:
> > On Tue, Nov 21, 2017 at 12:06:06PM +0100, Juergen Gross wrote:
> >> The "special pages" for PVH guests include the frames for console and
> >> Xenstore ring buffers. Those have to be marked as "Reserved" in the
> >> guest's E820 map, as otherwise conflicts might arise later e.g. when
> >> hotplugging memory into the guest.
> >>
> >> Signed-off-by: Juergen Gross 
> > 
> > Is this patch still relevant? The reasoning looks sensible to me fwiw.
> 
> I still think it should be applied, yes.

Acked + applied.

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

Re: [Xen-devel] [RFC PATCH 23/49] ARM: new VGIC: Add IRQ sorting

[Julien Grall]

Hi Andre,

On 09/02/18 14:39, 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 pulls in Linux' list_sort.c , which is a merge sort implementation
for linked lists.

This is based on Linux commit 8e4447457965, written by Christoffer Dall.

Signed-off-by: Andre Przywara 
---
  xen/arch/arm/vgic/vgic.c|  59 +++
  xen/common/list_sort.c  | 170 
  xen/include/xen/list_sort.h |  11 +++


You need to CC "THE REST" maintainers for this code. It would also make 
sense to have a separate patch for adding list_sort.c



  3 files changed, 240 insertions(+)
  create mode 100644 xen/common/list_sort.c
  create mode 100644 xen/include/xen/list_sort.h

diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c
index f517df6d00..a4efd1fd03 100644
--- a/xen/arch/arm/vgic/vgic.c
+++ b/xen/arch/arm/vgic/vgic.c
@@ -16,6 +16,7 @@
   */
  
  #include 

+#include 
  #include 
  
  #include 

@@ -163,6 +164,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".


Finally a good explanation of the return value of a sort function :). I 
always get confused what the return is supposed to be.



+ */
+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);


I guess the locking order does not matter here because this is the only 
place where two IRQs lock have to be taken?


Also, this will be done with irq disabled right? In that case, may I ask 
for an ASSERT(!local_irq_is_enabled())? Or maybe in vgic_sort_ap_list.



+
+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_cpu;
+
+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.
diff --git a/xen/common/list_sort.c b/xen/common/list_sort.c
new file mode 100644
index 00..9c5cc58e43
--- /dev/null
+++ b/xen/common/list_sort.c
@@ -0,0 +1,170 @@
+/*
+ * list_sort.c: merge sort implementation for linked lists
+ * Copied from the Linux kernel (lib/list_sort.c)
+ * (without specific copyright notice there)


I can see you moved from Linux to Xen coding style. Is there any other 
changes made?



+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; If not, see .
+ */
+#include 
+#include 
+
+#define MAX_LIST_LENGTH_BITS 20
+
+/*
+ * Returns a list organized in an intermediate format suited
+ * to chaining of merge() calls: null-terminated, no reserved or
+ * sentinel head node, "prev" links not maintained.
+ */
+static struct list_head *merge(void *priv,
+   int (*cmp)(void *priv, struct list_head *a,
+  struct list_head *b),
+   struct list_head *a, 

Re: [Xen-devel] [PATCH] x86/xen: Calculate __max_logical_packages on PV domains

[Ingo Molnar]

* Juergen Gross  wrote:

> On 08/02/18 01:59, Boris Ostrovsky wrote:
> > 
> > 
> > On 02/07/2018 06:49 PM, Prarit Bhargava wrote:
> >> The kernel panics on PV domains because native_smp_cpus_done() is
> >> only called for HVM domains.
> >>
> >> Calculate __max_logical_packages for PV domains.
> >>
> >> Fixes: b4c0a7326f5d ("x86/smpboot: Fix __max_logical_packages estimate")
> >> Signed-off-by: Prarit Bhargava 
> >> Tested-and-reported-by: Simon Gaiser 
> >> Cc: Thomas Gleixner 
> >> Cc: Ingo Molnar 
> >> Cc: "H. Peter Anvin" 
> >> Cc: x...@kernel.org
> >> Cc: Boris Ostrovsky 
> >> Cc: Juergen Gross 
> >> Cc: Dou Liyang 
> >> Cc: Prarit Bhargava 
> >> Cc: Kate Stewart 
> >> Cc: Greg Kroah-Hartman 
> >> Cc: Andy Lutomirski 
> >> Cc: Andi Kleen 
> >> Cc: Vitaly Kuznetsov 
> >> Cc: xen-devel@lists.xenproject.org
> > 
> > 
> > Reviewed-by: Boris Ostrovsky 
> 
> Thomas, Ingo, are you taking this via the tip tree or should I take
> it via the xen tree?

Since it's supposed to only affect Xen feel free to pick it up:

  Acked-by: Ingo Molnar 

Thanks,

Ingo

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

Re: [Xen-devel] Problem with IOMEM and domain reboot

[Wei Liu]

On Mon, Feb 12, 2018 at 07:22:27PM +0200, Oleksandr Grytsov wrote:
> On Wed, Feb 7, 2018 at 2:14 PM, Oleksandr Andrushchenko 
> wrote:
> 
> > On 02/06/2018 02:52 PM, Wei Liu wrote:
> >
> >> On Tue, Feb 06, 2018 at 02:44:56PM +0200, Oleksandr Andrushchenko wrote:
> >>
> >>>   From aa1f20af73a5a3c8f2c904b857a79334d18d41ff Mon Sep 17 00:00:00 2001
> >>>
>  From: Oleksandr Andrushchenko 
> > Date: Wed, 20 Dec 2017 17:51:18 +0200
> > Subject: [PATCH] [HACK] Reset iomem's gfn to LIBXL_INVALID_GFN on
> > reboot
> >
> > During domain reboot its configuration is partially reused
> > to re-create a new domain, but iomem's GFN field for the
> > iomem is only restored for those memory ranges, which are
> > configured in form of [IOMEM_START,NUM_PAGES[@GFN], but not for
> > those in form of [IOMEM_START,NUM_PAGES], e.g. without GFN.
> > For the latter GFN is reset to 0, but while mapping ranges
> > to a domain during reboot there is a check that GFN treated
> > as valid if it is not equal to LIBXL_INVALID_GFN, thus making
> > Xen to map IOMEM_START to address 0 in the guest's address space.
> >
> > Workaround it by resseting GFN to LIBXL_INVALID_GFN, so xl
> > can set proper values for mapping on reboot.
> >
> > Signed-off-by: Oleksandr Andrushchenko  > com>
> > ---
> >tools/libxl/libxl_domain.c | 9 +
> >1 file changed, 9 insertions(+)
> >
> > diff --git a/tools/libxl/libxl_domain.c b/tools/libxl/libxl_domain.c
> > index ef1a0927b00d..2678ad2ad54f 100644
> > --- a/tools/libxl/libxl_domain.c
> > +++ b/tools/libxl/libxl_domain.c
> > @@ -1647,6 +1647,15 @@ int libxl_retrieve_domain_configuration(libxl_ctx
> > *ctx, uint32_t domid,
> >}
> >}
> > +/* reset IOMEM's GFN to initial value */
> > +{
> > +int i;
> > +
> > +for (i = 0; i < d_config->b_info.num_iomem; i++)
> > +if (d_config->b_info.iomem[i].gfn == 0)
> > +d_config->b_info.iomem[i].gfn = LIBXL_INVALID_GFN;
> > +}
> > +
> >
>  I don't think this is necessary. Instead we should tell libxl to save
>  the generated value into the template. Add an update_config hook for the
>  iomem type should be better.
> 
> >>> Agree, this is why I tagged the patch as [HACK]
> >>> Unfortunately, I have little knowledge of libxl and not sure
> >>> how to properly fix it. Can you tell a bit more on what
> >>> a proper fix could be?
> >>>
> >> See libxl__update_domain_configuration, which is called after domain
> >> construction is completed. It will call the update_config hook for a
> >> device type to save anything that is generated in the process of domain
> >> creation. One example is in libxl_nic. You can do the same to iomem I
> >> think.
> >>
> >> The end result is the generated values you care about are saved into the
> >> template. When the domain is migrated / rebooted libxl will use the
> >> saved values instead.
> >>
> > Thank you, will look at it to make a proper fix
> >
> > Strictly speaking your patch of adding the snippet to
> >> libxl_retrieve_domain_configuration isn't wrong, but I would prefer that
> >> function to only contain code to fetch states that can be changed during
> >> domain runtime. The iomem range isn't one of those states AIUI.
> >>
> >> Wei.
> >>
> >>
> >> Wei.
> 
> >>> Thank you,
> >>> Oleksandr
> >>>
> >>
> >
> > ___
> > Xen-devel mailing list
> > Xen-devel@lists.xenproject.org
> > https://lists.xenproject.org/mailman/listinfo/xen-devel
> >
> 
> 
> Hi Wei,
> 
> The root cause of this problem is that auto generated code doesn't handle
> default value when json is parsed. It is related not only IOMEM but
> potentially some other structure as well.
> 

Oh, that. It most likely affect primitive types.

> Field "gfn" of libxl_iomem_range structure has default value
> LIBXL_INVALID_GFN
> which is not 0.
> 
> libxl_iomem_range = Struct("iomem_range", [
> ...
> ("gfn", uint64, {'init_val': "LIBXL_INVALID_GFN"}),
> ])
> 
> The default value is handled correctly when json is generated:
> 
> yajl_gen_status libxl_iomem_range_gen_json(yajl_gen hand, libxl_iomem_range
> *p)
> {
> ...
> 
> if (p->gfn != LIBXL_INVALID_GFN) {
> s = yajl_gen_string(hand, (const unsigned char *)"gfn",
> sizeof("gfn")-1);
> if (s != yajl_gen_status_ok)
> goto out;
> s = libxl__uint64_gen_json(hand, p->gfn);
> if (s != yajl_gen_status_ok)
> goto out;
> }
> 
> ...
> }
> 
> But when json is parsed, this "gfn" field is parsed as any other uint64
> value.
> As result we have 0 instead of LIBXL_INVALID_GFN.

Why? The above snippet says no output is generated if the value is
LIBXL_INVALID_GFN.

Hence ...

> 

Re: [Xen-devel] [PATCH] libxl: add libxl__is_driver_domain function

[Wei Liu]

On Tue, Feb 06, 2018 at 03:08:45PM +0200, Oleksandr Grytsov wrote:
> On Tue, Feb 6, 2018 at 2:36 PM, Wei Liu  wrote:
> 
> > On Thu, Dec 14, 2017 at 04:14:12PM +0200, Oleksandr Grytsov wrote:
> > > From: Oleksandr Grytsov 
> > >
> > > We have following arm-based setup:
> > >
> > > - Dom0 with xen and xen tools;
> > > - Dom1 with device backends (but it is not the driver domain);
> >
> > What is your definition of a "driver domain"? What does it do in this
> > case?
> >
> > I seem to have seen people use this term in different contexts to mean
> > slightly different things. I need to figure out what you actually mean
> > first.
> >
> >
> I see in the libxl/xl sources that closing PV devices is done differently
> in case backends are in Dom0 and are in other domain. It is called as
> driver domain in the sources. So, I don't have clear understanding
> what does it mean. In our setup backends are in Dom1 and xl is in Dom0.
> And I see that xl dosn't close PV device on domain reboot or shutdown.

Do you run xl devd in your backend domain?

Wei.

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

Re: [Xen-devel] [RFC PATCH 17/49] ARM: timer: Handle level triggered IRQs correctly

[Julien Grall]

On 12/02/18 18:23, Andre Przywara wrote:

Hi,


Hi Andre,


On 12/02/18 15:19, Julien Grall wrote:

Hi Andre,

On 09/02/18 14:39, 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 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.


You might want to make a summary of the discussion we had with Marc Z.
today here. This would help the other to understand why sample the
interrupt state is necessary :).


Yes, I just saw that I somehow missed copying the elaborate comment from
Christoffer. Fixed now, indeed without this background it's next to
impossible to understand this ;-)


Also do we need to do that for the emulated physical timer?


Mmh, good question. I believe this whole timer story needs a good think
again.



Signed-off-by: Andre Przywara 
---
   xen/arch/arm/time.c | 34 ++
   xen/arch/arm/traps.c    |  1 +
   xen/include/xen/timer.h |  2 ++
   3 files changed, 37 insertions(+)

diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index c11fcfeadd..98ebb4305d 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -263,6 +263,40 @@ static void vtimer_interrupt(int irq, void
*dev_id, struct cpu_user_regs *regs)
   vgic_inject_irq(current->domain, current,
current->arch.virt_timer.irq, true);
   }
   +/**


One * is enough.


That's kernel-doc comment style:
https://www.kernel.org/doc/html/v4.9/kernel-documentation.html#writing-kernel-doc-comments

That allows tools to scan the tree and extract function documentation in
an automated way. A bit like markdown: still perfectly readable by
humans, but parse-able by scripts as well.

I was hoping that it wouldn't hurt to have this in Xen as well, as I
copied this already in other parts of this code.


I was blindly following the CODING_STYLE requirements :). But let's keep 
/** if it helps script.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [RFC PATCH 15/49] ARM: GIC: Allow tweaking the active state of an IRQ

[Julien Grall]

On 12/02/18 17:53, Andre Przywara wrote:

Hi,


Hi Andre,


On 12/02/18 13:55, Julien Grall wrote:

Hi Andre,

On 09/02/18 14:39, Andre Przywara wrote:

When playing around with hardware mapped, level triggered virtual IRQs,
there is the need to explicitly set the active state of an interrupt at
some point in time.
To prepare the GIC for that, we introduce a set_active_state() function
to let the VGIC manipulate the state of an associated hardware IRQ.

Signed-off-by: Andre Przywara 
---
   xen/arch/arm/gic-v2.c |  9 +
   xen/arch/arm/gic-v3.c | 16 
   xen/arch/arm/gic.c    |  5 +
   xen/include/asm-arm/gic.h |  5 +
   4 files changed, 35 insertions(+)

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 2e35892881..5339f69fbc 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -235,6 +235,14 @@ static unsigned int gicv2_read_irq(void)
   return (readl_gicc(GICC_IAR) & GICC_IA_IRQ);
   }
   +static void gicv2_set_active_state(int irq, bool active)


I would much prefer to have an irq_desc in parameter. This is matching
the other interface


... and that's why I had it just like this in my first version. However
this proved to be nasty because I now need to get this irq_desc pointer
first, as the caller doesn't have it already. Since all we have and need
is the actual hardware IRQ number, I found it more straight-forward to
just use that number directly instead of going via the pointer and back
(h/w intid => irq_desc => irq).


and you could update the flags such as
_IRQ_INPROGRESS which you don't do at the moment.


Mmh, interesting point. I guess I should also clear this bit in the new
VGIC. At least once I wrapped my head around what this flag is
*actually* for (in conjunction with _IRQ_GUEST).
Anyway I guess this bit would still be set in our case.


For IRQ routed to the guest, the flag is used to know whether you need 
to EOI the interrupt on domain destruction.


In general, I would like to keep desc->status in sync for the guest IRQ. 
This is useful for debugging and potentially some ratelimit on interrupt 
(I am thinking for ITS).





Also, who is preventing two CPUs to clear the active bit at the same time?


A certain hardware IRQ is assigned to one virtual IRQ on one VCPU at one
time only. Besides, GICD_ICACTIVERn has wired NAND semantics, so that's
naturally race free (as it was designed to be).
Unless I miss something here (happy to be pointed to an example where it
causes problems).


You could potentially have a race between ICACTIVER an ISACTIVER. This 
is very similar to the enable/disable part. This matters a lot when 
updating desc->status.



+}
+
   static void gicv2_set_irq_type(struct irq_desc *desc, unsigned int
type)
   {
   uint32_t cfg, actual, edgebit;
@@ -1241,6 +1249,7 @@ 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_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 08d4703687..595eaef43a 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -475,6 +475,21 @@ static unsigned int gicv3_read_irq(void)
   return irq;
   }
   +static void gicv3_set_active_state(int irq, bool active)
+{
+    void __iomem *base;
+
+    if ( irq >= NR_GIC_LOCAL_IRQS)
+    base = GICD + (irq / 32) * 4;
+    else
+    base = GICD_RDIST_SGI_BASE;
+
+    if ( active )
+    writel(1U << (irq % 32), base + GICD_ISACTIVER);
+    else
+    writel(1U << (irq % 32), base + GICD_ICACTIVER);


Shouldn't you wait until RWP bits is cleared here?


I don't see why. I think this action has some posted semantics anyway,
so no need for any synchronisation. And also RWP does not track
I[SC]ACTIVER, only ICENABLER and some CTLR bits (ARM IHI 0069D, 8.9.4:
RWP[31]).




+}


Why don't you use the function poke?


Ah, I didn't see this. But then this now does this quite costly RWP
dance now. We could add a check in there to only do this if we change
the affected registers or pass an explicit "bool wait_for_rwp" in there.


I guess this would be useful even for the current code. If I understand 
correctly the RWP semantics, it should not be necessary to wait when 
write to ISENABLER also.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2] tools/libxl: mark special pages as reserved in e820 map for PVH

[Juergen Gross]

On 13/02/18 12:55, Wei Liu wrote:
> On Tue, Nov 21, 2017 at 12:06:06PM +0100, Juergen Gross wrote:
>> The "special pages" for PVH guests include the frames for console and
>> Xenstore ring buffers. Those have to be marked as "Reserved" in the
>> guest's E820 map, as otherwise conflicts might arise later e.g. when
>> hotplugging memory into the guest.
>>
>> Signed-off-by: Juergen Gross 
> 
> Is this patch still relevant? The reasoning looks sensible to me fwiw.

I still think it should be applied, yes.


Juergen

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

Re: [Xen-devel] [RFC PATCH 09/49] ARM: VGIC: change to level-IRQ compatible IRQ injection interface

[Julien Grall]

Hi,

On 12/02/18 14:24, Andre Przywara wrote:

What would you expect the caller to do on error? Except printing an
error message?


I don't know either. Comparing this to hardware, an IRQ is usually
fire-and-forget (separating the interrupt line from the interrupt state
here), so a device doesn't really handle the case when an IRQ does not
make it through (it can't know easily anyway). However the whole state
machine might get busted in the process (if no one lowers the line, for
instance).
So looking at this printing a message looks like the best choice.

I checked all users of vgic_inject_irq(), at the moment all IRQ numbers
passed in look safe: they are either hardcoded (timer, evtchn) or
validated before (hardware IRQs, when they are tied to a virtual IRQ).
So indeed we *should* never see an invalid IRQ number, at the moment.
I need to check how this changes with the ITS, though.

So we could change the prototype (back) to void, but print some error
message if the vgic_get_irq() call fails within vgic_inject_irq().


Sounds good to me. Make sure to have those message using the log level 
guest debug message. I might even be tempt to use dgprintk(...) here so 
they get dropped in non-debug build.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v3 00/17] Alternative Meltdown mitigation

[Juergen Gross]

On 12/02/18 18:54, Dario Faggioli wrote:
> On Fri, 2018-02-09 at 15:01 +0100, Juergen Gross wrote:
>> This series is available via github:
>>
>> https://github.com/jgross1/xen.git xpti
>>
>> Dario wants to do some performance tests for this series to compare
>> performance with Jan's series with all optimizations posted.
>>
> And some of this is indeed ready.
> 
> So, this is again on my testbox, with 16 pCPUs and 12GB of RAM, and I
> used a guest with 16 vCPUs and 10GB of RAM.
> 
> I benchmarked Jan's patch *plus* all the optimizations and overhead
> mitigation patches he posted on xen-devel (the ones that are already in
> staging, and also the ones that are not yet there). That's "XPTI-Light" 
> in the table and in the graphs. Booting this with 'xpti=false' is
> considered the baseline, while booting with 'xpti=true' is the actual
> thing we want to measure. :-)
> 
> Then I ran the same benchmarks on Juergen's branch above, enabled at
> boot. That's "XPYI" in the table and graphs (yes, I know, sorry for the
> typo!).
> 
> http://openbenchmarking.org/result/1802125-DARI-180211144
> http://openbenchmarking.org/result/1802125-DARI-180211144_hgv=XPTI-Light+xpti%3Dfalse_nor=y_hgv=XPTI-Light+xpti%3Dfalse

...

> Or, actually, that's not it! :-O In fact, right while I was writing
> this report, it came out on IRC that something can be done, on
> Juergen's XPTI series, to mitigate the performance impact a bit.
> 
> Juergen sent me a patch already, and I'm re-running the benchmarks with
> that applied. I'll let know how the results ends up looking like.

It turned out the results are not basically different. So the general
problem with context switches is still there (which I expected, BTW).

So I guess the really bad results with benchmarks triggering a lot of
vcpu scheduling show that my approach isn't going to fly, as the most
probable cause for the slow context switches are the introduced
serializing instructions (LTR, WRMSRs) which can't be avoided when we
want to use per-vcpu stacks.

OTOH the results of the other benchmarks showing some advantage over
Jan's solution indicate there is indeed an aspect which can be improved.

Instead of preferring one approach over the other I have thought about
a way to use the best parts of each solution in a combined variant. In
case nobody is feeling strong to pursue my current approach further I'd
like to suggest the following scheme:

- Whenever a L4 page table of the guest is in use on one physical cpu
  only use the L4 shadow cache of my series in order to avoid having to
  copy the L4 contents each time the hypervisor is left.

- As soon as a L4 page table is being activated on a second cpu fall
  back to use the per-cpu page table on that cpu (the cpu already using
  the L4 page table can continue doing so).

- Before activation of a L4 shadow page table it is modified to map the
  per-cpu data needed in guest mode for the local cpu only.

- Use INVPCID instead of %cr4 PGE toggling to speed up purging global
  TLB entries (depending on the availability of the feature, of course).

- Use the PCID feature for being able to avoid purging TLB entries which
  might be needed later (depending on hardware again). I expect this
  will help especially for cases where the guest often switches between
  kernel and user mode. Whether we want 3 or 4 PCID values for each
  guest address space has to be discussed: do we need 2 different Xen
  variants for guest user and guest kernel (IOW: are there any problems
  possible when the hypervisor is using a guest kernel's permission to
  access guest data when the guest was running in user mode before
  entering the hypervisor)?

Thoughts?


Juergen

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

Re: [Xen-devel] [Minios-devel] [PATCH] mini-os: add a coding style file

[Wei Liu]

On Mon, Feb 12, 2018 at 11:59:23AM +0100, Juergen Gross wrote:
> On 09/11/17 13:45, Wei Liu wrote:
> > On Thu, Nov 09, 2017 at 01:35:49PM +0100, Juergen Gross wrote:
> >> On 09/11/17 13:31, Wei Liu wrote:
> >>> On Thu, Nov 09, 2017 at 01:10:12PM +0100, Juergen Gross wrote:
>  Since carving out Mini-OS from the Xen repository there hasn't been a
>  description of the preferred coding style. Copy the Xen CODING_STYLE
>  file.
> 
> >>>
> >>> I welcome such addition. I have no opinion in actual style used though.
> >>> I just want consistency. :-)
> >>
> >> Is this an Ack?
> >>
> > 
> > Yes.
> > 
> > Acked-by: Wei Liu 
> 
> So this is pending for 3 months now...
> 

Oops, sorry. You know why I didn't get to this earlier. :-)

This and other pending patches will be committed shortly.

Wei.

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

Re: [Xen-devel] [PATCH RFC 2/8] public/io/netif: add directory for backend parameters

[Wei Liu]

On Thu, Feb 08, 2018 at 01:51:28PM +, Joao Martins wrote:
> > We can probably specify a xenstore node in the spec to
> > return some error code and let libxl read it. With that model old tools
> > work the same (extra node ignored) but new tools can utilise the new
> > node. IIRC there could already be some node that can be utilised --
> > xenbus_dev_fatal writes message to xenstore, I think.
> > 
> 
> I almost forgot about xenbus_dev_fatal(). It writes to an "error" entry in the
> backend|frontend path with the errno plus error message. But it also changes 
> the
> device xenbus state to XenbusClosed. Taking into consideration your earlier
> comment you probably meant xenbus_dev_error() instead? netback does allow
> Initialising state to be directly into Closing, but others might not be the 
> same.
> 

Yep, xenbus_dev_error is probably better.

> > What do you think?
> 
> I like the idea of having a similar "error" entry in the config|require
> directory following the same pattern as mentioned in the last paragraph.
> 
> Something like:
> 
> /config/error = " "
> 
> I would imagine this could be wrapper in a xenbus_config_fatal().
> 
> I had suggested a slightly more complicated version of it in a old reply to 
> Paul
> (at top of this message) with:
> 
> /require/- = ""
> /require/-status = ""
> 
> But taking morphing it with your comment could also be something like:
> 
> /config/ = ""
> /config//error = " "
> 
> But either this option or the config/error global one depend on whether people
> here prefer to deliver all configuration errors at once, or one global
> "config/error" which would give the first entry with an error. The latter 
> though
> could lead to the sysadmin having to recreate the domain multiple times to
> see/handle all the errors.
> 

I'm not too opinionated on this really. I just want things to be
properly documented, cover known use cases well and allow for future
extensions.

Wei.

>   Joao
> 
> P.S. s/require/config

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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf-pvopsbroken
 build-i386-libvirt6 libvirt-buildfail REGR. vs. 118698
 test-amd64-amd64-xl-qemuu-ovmf-amd64 16 guest-localmigrate/x10 fail REGR. vs. 
118698
 build-armhf-pvops 5 host-build-prep  fail REGR. vs. 118698

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

version targeted for testing:
 xen  8b1a5268daf0ff1ddca49d2e683e5bfabf6b9988
baseline version:
 xen  c93014ad3aa6aa88dfa5e96f66e8adb561483b8d

Last test of basis   118698  2018-02-08 19:23:11 Z4 days
Failing since118802  2018-02-10 00:36:18 Z3 days4 attempts
Testing same since   119023  2018-02-12 21:23:25 Z0 days1 attempts


People who touched revisions under test:
  Andre Przywara 
  Andrew Cooper 
  George Dunlap 
  Haozhong Zhang 
  Jan Beulich 
  Julien Grall 
  Julien Grall 
  Kevin Tian 
  Roger Pau Monne 
  Roger Pau Monné 
  Stefano Stabellini 
  Wei Liu 

jobs:
 build-amd64-xsm  pass
 

Re: [Xen-devel] [PATCH 3/3] pvh/dom0: whitelist PVH Dom0 ACPI tables

[Roger Pau Monné]

On Tue, Feb 13, 2018 at 04:04:17AM -0700, Jan Beulich wrote:
> >>> On 13.02.18 at 10:59,  wrote:
> > On Tue, Feb 13, 2018 at 02:29:08AM -0700, Jan Beulich wrote:
> >> >>> On 08.02.18 at 13:25,  wrote:
> >> > Signed-off-by: Roger Pau Monné 
> >> 
> >> A change like this should not come without description, providing a
> >> reason for the change. Otherwise how will someone wanting to
> >> understand the change in a couple of years actually be able to
> >> make any sense of it. This is in particular because I continue to be
> >> not fully convinced that white listing is appropriate in the Dom0
> >> case (and for the record I'm similarly unconvinced that black listing
> >> is the best choice, yet obviously we need to pick on of the two).
> > 
> > I'm sorry, I thought we agreed at the summit to convert this to
> > whitelisting because it was likely better to simply not expose unknown
> > ACPI tables to guests.
> 
> "to guests" != "to Dom0".
> 
> > I guess the commit message could be something like:
> > 
> > "The following list of whitelisted APIC tables are either known to work
> > or don't require any resources to be mapped in either the IO or the
> > memory space.
> 
> Even if the white listing vs black listing question wasn't still
> undecided, I think we should revert the patch in favor of one
> with a description. The one above might be fine with "ACPI" in
> place of "APIC" as far as tables actively white listed are
> concerned, but then it still remains open why certain tables
> haven't been included. I'm in particular worried about various
> APEI related tables, but invisibility of e.g. an IBFT could also
> lead to boot problems.

Regarding APEI I think ERST, EINJ and HEST could be passed through,
BERT however requires that the BOOT Error Region is mapped into Dom0
p2m.

Since PVH Dom0 creation still ends up in a panic, I see no problem in
adding those in follow up patches.

IBFT also looks safe to pass through.

Thanks, Roger.

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

Re: [Xen-devel] [RFC XEN PATCH v4 00/41] Add vNVDIMM support to HVM domains

[Roger Pau Monné]

On Tue, Feb 13, 2018 at 04:05:45AM -0700, Jan Beulich wrote:
> >>> On 13.02.18 at 11:29,  wrote:
> > On Tue, Feb 13, 2018 at 03:06:24AM -0700, Jan Beulich wrote:
> >> >>> On 12.02.18 at 11:05,  wrote:
> >> > If you map the NVDIMM as MMIO to Dom0 you don't need the M2P entries
> >> > IIRC, and if it's mapped using 1GB pages it shouldn't use that much
> >> > memory for the page tables (ie: you could just use normal RAM for the
> >> > page tables that map the NVDIMM IMO). Of course that only applies to
> >> > PVH/HVM.
> >> 
> >> But in order to use (part of) it in a RAM-like manner we need struct
> >> page_info for it.
> > 
> > I guess the main use of this would be to grant NVDIMM pages? And
> > without a page_info that's not possible.
> 
> Why grant? Simply giving such a page as RAM to a guest would
> already be a problem without struct page_info (as then we can't
> track the page owner, nor can we refcount the page).

My point was to avoid doing that, and always assign the pages as
MMIO, which IIRC doesn't require a struct page_info.

Roger.

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

Re: [Xen-devel] [PATCH 4/7] x86/asm: Remove opencoded uses of altinstruction_entry

[Jan Beulich]

>>> On 13.02.18 at 11:07,  wrote:
> On 13/02/2018 09:56, Jan Beulich wrote:
> On 12.02.18 at 13:30,  wrote:
>>> On Mon, Feb 12, 2018 at 11:23:04AM +, Andrew Cooper wrote:
 diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
 index 58f652d..bd3819a 100644
 --- a/xen/arch/x86/x86_64/entry.S
 +++ b/xen/arch/x86/x86_64/entry.S
 @@ -557,23 +557,9 @@ handle_exception_saved:
  testb $X86_EFLAGS_IF>>8,UREGS_eflags+1(%rsp)
  jzexception_with_ints_disabled
  
 -.Lcr4_pv32_orig:
 -jmp   .Lcr4_pv32_done
 -.skip (.Lcr4_pv32_alt_end - .Lcr4_pv32_alt) - (. - 
 .Lcr4_pv32_orig), 0xcc
 -.pushsection .altinstr_replacement, "ax"
 -.Lcr4_pv32_alt:
 -mov   VCPU_domain(%rbx),%rax
 -.Lcr4_pv32_alt_end:
 -.section .altinstructions, "a"
 -altinstruction_entry .Lcr4_pv32_orig, .Lcr4_pv32_alt, \
 - X86_FEATURE_XEN_SMEP, \
 - (.Lcr4_pv32_alt_end - .Lcr4_pv32_alt), \
 - (.Lcr4_pv32_alt_end - .Lcr4_pv32_alt)
 -altinstruction_entry .Lcr4_pv32_orig, .Lcr4_pv32_alt, \
 - X86_FEATURE_XEN_SMAP, \
 - (.Lcr4_pv32_alt_end - .Lcr4_pv32_alt), \
 - (.Lcr4_pv32_alt_end - .Lcr4_pv32_alt)
 -.popsection
 +ALTERNATIVE_2 "jmp .Lcr4_pv32_done; .skip 2, 0x90", \
>>> This changed 0xcc to 0x90 but since it is just padding following an
>>> unconditional jmp so it shouldn't matter.
>> Well, it was for that very reason that I had picked 0xcc originally:
>> We better know if some branch mistakenly leads into that region.
> 
> Know how?  At the time you wrote this, Xen silently executed its way
> through debug traps, and it took some persuading to get you to ok the
> patch which at least printed a line every time we a breakpoint in
> hypervisor space.

Granted I didn't realize at the time that breakpoints would go all
silent.

> If you actually want to notice going down the wrong path, then you want
> a BUG.

I'd be very much in favor of this, if only there was a single byte insn
documented to cause #UD now and forever. Abusing what is INTO or
SALC outside of 64-bit mode doesn't look very attractive.

>> I also very much object to the literal 2 passed as an argument to
>> .skip above: What if the label moves out far enough that a short
>> branch won't be usable anymore?
> 
> Is the commit message not enough?  a) its not going to change, because
> it hasn't changed since you put the code in originally and I don't
> expect it to in the future, and b) it is a temporary necessary
> requirement to make the series bisectable and reviewable.  This skip is
> dropped in patch 6 when the automatic padding calculations work.

Oh, if it goes away by the end of the series, then that's fine.
(When replying here I hadn't looked at the full patch yet, so please
accept my apologies if this is properly explained in the description.)

Jan


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

Re: [Xen-devel] [PATCH 7/7] x86/build: Use new .nop directive when available

[Roger Pau Monné]

On Mon, Feb 12, 2018 at 11:23:07AM +, Andrew Cooper wrote:
> Newer versions of binutils are capable of emitting an exact number bytes worth
> of optimised nops.  Use this in preference to .skip when available.
> 
> Signed-off-by: Andrew Cooper 
> ---
> CC: Jan Beulich 
> CC: Konrad Rzeszutek Wilk 
> CC: Roger Pau Monné 
> CC: Wei Liu 
> 
> RFC until support is actually committed to binutils mainline.

Do you think you could reference the documentation? I can guess what
the arguments are, but would be better if I can have a reference.

> ---
>  xen/arch/x86/Rules.mk |  1 +
>  xen/include/asm-x86/alternative-asm.h | 16 +---
>  xen/include/asm-x86/alternative.h | 17 -
>  3 files changed, 26 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
> index 56b2ea8..c3b726c 100644
> --- a/xen/arch/x86/Rules.mk
> +++ b/xen/arch/x86/Rules.mk
> @@ -20,6 +20,7 @@ $(call as-insn-check,CFLAGS,CC,"invept 
> (%rax)$$(comma)%rax",-DHAVE_GAS_EPT)
>  $(call as-insn-check,CFLAGS,CC,"rdrand %eax",-DHAVE_GAS_RDRAND)
>  $(call as-insn-check,CFLAGS,CC,"rdfsbase %rax",-DHAVE_GAS_FSGSBASE)
>  $(call as-insn-check,CFLAGS,CC,"rdseed %eax",-DHAVE_GAS_RDSEED)
> +$(call as-insn-check,CFLAGS,CC,".nop 0$$(comma)9",-DHAVE_GAS_LONG_NOPS)

Since .nop is going to be used with labels, might be better to have a
test like:

.L1:
.L2:
.nop (.L2 - .L1),9

At least with clang assembler some expressions used in directives
allow labels, while others not (ie: .skip and .fill allow labels,
.rept doesn't)

>  $(call as-insn-check,CFLAGS,CC,".equ \"x\"$$(comma)1", \
>   -U__OBJECT_LABEL__ -DHAVE_GAS_QUOTED_SYM \
>   '-D__OBJECT_LABEL__=$(subst 
> $(BASEDIR)/,,$(CURDIR))/$$@')
> diff --git a/xen/include/asm-x86/alternative-asm.h 
> b/xen/include/asm-x86/alternative-asm.h
> index f7e37cb..a4893e7 100644
> --- a/xen/include/asm-x86/alternative-asm.h
> +++ b/xen/include/asm-x86/alternative-asm.h
> @@ -1,6 +1,8 @@
>  #ifndef _ASM_X86_ALTERNATIVE_ASM_H_
>  #define _ASM_X86_ALTERNATIVE_ASM_H_
>  
> +#include 
> +
>  #ifdef __ASSEMBLY__
>  
>  /*
> @@ -18,6 +20,14 @@
>  .byte \pad_len
>  .endm
>  
> +.macro mknops nr_bytes
> +#ifdef HAVE_GAS_LONG_NOPS
> +.nop \nr_bytes, ASM_NOP_MAX
> +#else
> +.skip \nr_bytes, 0x90
> +#endif
> +.endm

Out of curiosity, is there any reason this is an assembly macro
instead of a define?

Easier to integrate with existing code?

Thanks, Roger.

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

Re: [Xen-devel] [RFC XEN PATCH v4 00/41] Add vNVDIMM support to HVM domains

[Jan Beulich]

>>> On 13.02.18 at 11:29,  wrote:
> On Tue, Feb 13, 2018 at 03:06:24AM -0700, Jan Beulich wrote:
>> >>> On 12.02.18 at 11:05,  wrote:
>> > If you map the NVDIMM as MMIO to Dom0 you don't need the M2P entries
>> > IIRC, and if it's mapped using 1GB pages it shouldn't use that much
>> > memory for the page tables (ie: you could just use normal RAM for the
>> > page tables that map the NVDIMM IMO). Of course that only applies to
>> > PVH/HVM.
>> 
>> But in order to use (part of) it in a RAM-like manner we need struct
>> page_info for it.
> 
> I guess the main use of this would be to grant NVDIMM pages? And
> without a page_info that's not possible.

Why grant? Simply giving such a page as RAM to a guest would
already be a problem without struct page_info (as then we can't
track the page owner, nor can we refcount the page).

Jan


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

Re: [Xen-devel] [PATCH 3/3] pvh/dom0: whitelist PVH Dom0 ACPI tables

[Jan Beulich]

>>> On 13.02.18 at 10:59,  wrote:
> On Tue, Feb 13, 2018 at 02:29:08AM -0700, Jan Beulich wrote:
>> >>> On 08.02.18 at 13:25,  wrote:
>> > Signed-off-by: Roger Pau Monné 
>> 
>> A change like this should not come without description, providing a
>> reason for the change. Otherwise how will someone wanting to
>> understand the change in a couple of years actually be able to
>> make any sense of it. This is in particular because I continue to be
>> not fully convinced that white listing is appropriate in the Dom0
>> case (and for the record I'm similarly unconvinced that black listing
>> is the best choice, yet obviously we need to pick on of the two).
> 
> I'm sorry, I thought we agreed at the summit to convert this to
> whitelisting because it was likely better to simply not expose unknown
> ACPI tables to guests.

"to guests" != "to Dom0".

> I guess the commit message could be something like:
> 
> "The following list of whitelisted APIC tables are either known to work
> or don't require any resources to be mapped in either the IO or the
> memory space.

Even if the white listing vs black listing question wasn't still
undecided, I think we should revert the patch in favor of one
with a description. The one above might be fine with "ACPI" in
place of "APIC" as far as tables actively white listed are
concerned, but then it still remains open why certain tables
haven't been included. I'm in particular worried about various
APEI related tables, but invisibility of e.g. an IBFT could also
lead to boot problems.

Andrew - you've acked and committed this, what is your opinion
here?

Jan

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

Re: [Xen-devel] [PATCH] x86/srat: fix the end pfn check in valid_numa_range()

[Jan Beulich]

>>> On 12.02.18 at 02:44,  wrote:
> --- a/xen/arch/x86/srat.c
> +++ b/xen/arch/x86/srat.c
> @@ -110,8 +110,8 @@ int valid_numa_range(u64 start, u64 end, nodeid_t node)
>   for (i = 0; i < num_node_memblks; i++) {
>   struct node *nd = _memblk_range[i];
>  
> - if (nd->start <= start && nd->end > end &&
> - memblk_nodeid[i] == node )
> + if (nd->start <= start && nd->end >= end &&
> + memblk_nodeid[i] == node)
>   return 1;
>   }
>  

You also should have fixed the indentation issue on the second
line of the if().

There looks to be a similar issue in nodes_cover_memory(), and I
think it would be rather desirable if a change here dealt with all
such issues rather than just picking one. Anyway - I'll produce a
patch for this second issue.

Jan


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

Re: [Xen-devel] [RFC XEN PATCH v4 00/41] Add vNVDIMM support to HVM domains

[Roger Pau Monné]

On Tue, Feb 13, 2018 at 03:06:24AM -0700, Jan Beulich wrote:
> >>> On 12.02.18 at 11:05,  wrote:
> > If you map the NVDIMM as MMIO to Dom0 you don't need the M2P entries
> > IIRC, and if it's mapped using 1GB pages it shouldn't use that much
> > memory for the page tables (ie: you could just use normal RAM for the
> > page tables that map the NVDIMM IMO). Of course that only applies to
> > PVH/HVM.
> 
> But in order to use (part of) it in a RAM-like manner we need struct
> page_info for it.

I guess the main use of this would be to grant NVDIMM pages? And
without a page_info that's not possible.

Roger.

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

Re: [Xen-devel] [PATCH 5/7] x86/alt: Support for automatic padding calculations

[Roger Pau Monné]

On Tue, Feb 13, 2018 at 10:09:15AM +, Andrew Cooper wrote:
> On 13/02/2018 09:45, Roger Pau Monné wrote:
> > On Mon, Feb 12, 2018 at 11:23:05AM +, Andrew Cooper wrote:
> >>  .macro ALTERNATIVE oldinstr, newinstr, feature
> >>  .L\@_orig_s:
> >>  \oldinstr
> >>  .L\@_orig_e:
> >> + .skip (-((repl_len(1) - orig_len) > 0) * (repl_len(1) - orig_len)), 
> >> 0x90
> > clang chokes on this expression, because of the negation at the
> > beginning and I'm also failing to see why are you adding such
> > negation. AFAICT using:
> >
> > .skip (((repl_len(1) - orig_len) > 0) * (repl_len(1) - orig_len)), 0x90
> >
> > Is correct: it adds the right padding if the alternative code is
> > bigger than the original one, while not adding anything is the
> > original code is greater than the alternative one.
> >
> > The negation just turns the 1 to -1, thus converting the result of the
> > whole expression into a negative value.
> 
> /sigh so Clang and GAS have different ideas of true.
> 
> The reason for this negation is stated in the commit message.  "x > 0"
> in GAS yields 0 or -1, rather than the expected 1.

That's unfortunate. What about something along the lines of:

---8<---
diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
index aeae01cd97..db442a45b7 100644
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -23,6 +23,7 @@ $(call as-insn-check,CFLAGS,CC,"rdseed 
%eax",-DHAVE_GAS_RDSEED)
 $(call as-insn-check,CFLAGS,CC,".equ \"x\"$$(comma)1", \
  -U__OBJECT_LABEL__ -DHAVE_GAS_QUOTED_SYM \
  '-D__OBJECT_LABEL__=$(subst $(BASEDIR)/,,$(CURDIR))/$$@')
+$(call as-insn-check,CFLAGS,CC,".skip (-(1 > 
0))$$(comma)0x90",-DAS_NEGATIVE_TRUE)
 
 CFLAGS += -mno-red-zone -fpic -fno-asynchronous-unwind-tables
 
diff --git a/xen/include/asm-x86/alternative-asm.h 
b/xen/include/asm-x86/alternative-asm.h
index f7e37cb891..6ce6479e5b 100644
--- a/xen/include/asm-x86/alternative-asm.h
+++ b/xen/include/asm-x86/alternative-asm.h
@@ -25,11 +25,18 @@
 #define decl_repl(insn, nr) .L\@_repl_s\()nr: insn; .L\@_repl_e\()nr:
 #define gas_max(a, b)  ((a) ^ (((a) ^ (b)) & -(-((a) < (b)
 
+#ifdef AS_NEGATIVE_TRUE
+#define as_true -
+#else
+#define as_true
+#endif
+
 .macro ALTERNATIVE oldinstr, newinstr, feature
 .L\@_orig_s:
 \oldinstr
 .L\@_orig_e:
- .skip (-((repl_len(1) - orig_len) > 0) * (repl_len(1) - orig_len)), 0x90
+ .skip (as_true((repl_len(1) - orig_len) > 0) * (repl_len(1) - orig_len)), 
\
+   0x90
 .L\@_orig_p:
 
 .pushsection .altinstructions, "a", @progbits
@@ -56,8 +63,8 @@
 .L\@_orig_s:
 \oldinstr
 .L\@_orig_e:
-.skip (-((gas_max(repl_len(1), repl_len(2)) - orig_len) > 0) * \
- (gas_max(repl_len(1), repl_len(2)) - orig_len)), 0x90
+.skip (as_true((gas_max(repl_len(1), repl_len(2)) - orig_len) > 0) * \
+   (gas_max(repl_len(1), repl_len(2)) - orig_len)), 0x90
 .L\@_orig_p:
 
 .pushsection .altinstructions, "a", @progbits
diff --git a/xen/include/asm-x86/alternative.h 
b/xen/include/asm-x86/alternative.h
index 20dea2245a..ea76fa9f8d 100644
--- a/xen/include/asm-x86/alternative.h
+++ b/xen/include/asm-x86/alternative.h
@@ -37,19 +37,25 @@ extern void alternative_instructions(void);
 #define gas_max(a, b) \
 "((" a ") ^ (((" a ") ^ (" b ")) & -(-((" a ") < (" b ")"
 
-#define OLDINSTR_1(oldinstr, n1)  \
-".L%=_orig_s:\n\t" oldinstr "\n .L%=_orig_e:\n\t" \
-".skip (-(("alt_repl_len(n1)"-"alt_orig_len") > 0) * "\
- "("alt_repl_len(n1)"-"alt_orig_len")), 0x90\n\t" \
+#ifdef AS_NEGATIVE_TRUE
+#define as_true -
+#else
+#define as_true
+#endif
+
+#define OLDINSTR_1(oldinstr, n1)\
+".L%=_orig_s:\n\t" oldinstr "\n .L%=_orig_e:\n\t"   \
+".skip ("as_true"(("alt_repl_len(n1)"-"alt_orig_len") > 0) * "  \
+ "("alt_repl_len(n1)"-"alt_orig_len")), 0x90\n\t"   \
 ".L%=_orig_p:\n\t"
 
 #define ALT_PADDING_LEN(n1, n2) \
 gas_max((alt_repl_len(n1), alt_repl_len(n2))"-"alt_orig_len
 
-#define OLDINSTR_2(oldinstr, n1, n2)  \
-".L%=_orig_s:\n\t" oldinstr "\n .L%=_orig_e:\n\t" \
-".skip (-(("ALT_PADDING_LEN(n1, n2)") > 0) * "\
- "("ALT_PADDING_LEN(n1, n2)")), 0x90\n\t" \
+#define OLDINSTR_2(oldinstr, n1, n2)\
+".L%=_orig_s:\n\t" oldinstr "\n .L%=_orig_e:\n\t"   \
+".skip ("as_true"(("ALT_PADDING_LEN(n1, n2)") > 0) * "  \
+ "("ALT_PADDING_LEN(n1, n2)")), 0x90\n\t"   \
 ".L%=_orig_p:\n\t"
 
 #define ALTINSTR_ENTRY(feature, num)\


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

Re: [Xen-devel] [PATCH v3 02/17] x86: do a revert of e871e80c38547d9faefc6604532ba3e985e65873

[Jan Beulich]

>>> On 09.02.18 at 15:01,  wrote:
> Revert "x86: allow Meltdown band-aid to be disabled" in order to
> prepare for a final Meltdown mitigation.

This no also reverts a22320e32dca0918ed23799583f470afe4c24330
afaict. I think that it would be better to revert the whole thing in a
single patch anyway (i.e. also fold patch 3 into here).

Jan


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

Re: [Xen-devel] [PATCH 5/7] x86/alt: Support for automatic padding calculations

[Andrew Cooper]

On 13/02/2018 09:45, Roger Pau Monné wrote:
> On Mon, Feb 12, 2018 at 11:23:05AM +, Andrew Cooper wrote:
>>  .macro ALTERNATIVE oldinstr, newinstr, feature
>>  .L\@_orig_s:
>>  \oldinstr
>>  .L\@_orig_e:
>> + .skip (-((repl_len(1) - orig_len) > 0) * (repl_len(1) - orig_len)), 
>> 0x90
> clang chokes on this expression, because of the negation at the
> beginning and I'm also failing to see why are you adding such
> negation. AFAICT using:
>
> .skip (((repl_len(1) - orig_len) > 0) * (repl_len(1) - orig_len)), 0x90
>
> Is correct: it adds the right padding if the alternative code is
> bigger than the original one, while not adding anything is the
> original code is greater than the alternative one.
>
> The negation just turns the 1 to -1, thus converting the result of the
> whole expression into a negative value.

/sigh so Clang and GAS have different ideas of true.

The reason for this negation is stated in the commit message.  "x > 0"
in GAS yields 0 or -1, rather than the expected 1.

~Andrew

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

Re: [Xen-devel] [PATCH 4/7] x86/asm: Remove opencoded uses of altinstruction_entry

[Andrew Cooper]

On 13/02/2018 09:56, Jan Beulich wrote:
 On 12.02.18 at 13:30,  wrote:
>> On Mon, Feb 12, 2018 at 11:23:04AM +, Andrew Cooper wrote:
>>> diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
>>> index 58f652d..bd3819a 100644
>>> --- a/xen/arch/x86/x86_64/entry.S
>>> +++ b/xen/arch/x86/x86_64/entry.S
>>> @@ -557,23 +557,9 @@ handle_exception_saved:
>>>  testb $X86_EFLAGS_IF>>8,UREGS_eflags+1(%rsp)
>>>  jzexception_with_ints_disabled
>>>  
>>> -.Lcr4_pv32_orig:
>>> -jmp   .Lcr4_pv32_done
>>> -.skip (.Lcr4_pv32_alt_end - .Lcr4_pv32_alt) - (. - 
>>> .Lcr4_pv32_orig), 0xcc
>>> -.pushsection .altinstr_replacement, "ax"
>>> -.Lcr4_pv32_alt:
>>> -mov   VCPU_domain(%rbx),%rax
>>> -.Lcr4_pv32_alt_end:
>>> -.section .altinstructions, "a"
>>> -altinstruction_entry .Lcr4_pv32_orig, .Lcr4_pv32_alt, \
>>> - X86_FEATURE_XEN_SMEP, \
>>> - (.Lcr4_pv32_alt_end - .Lcr4_pv32_alt), \
>>> - (.Lcr4_pv32_alt_end - .Lcr4_pv32_alt)
>>> -altinstruction_entry .Lcr4_pv32_orig, .Lcr4_pv32_alt, \
>>> - X86_FEATURE_XEN_SMAP, \
>>> - (.Lcr4_pv32_alt_end - .Lcr4_pv32_alt), \
>>> - (.Lcr4_pv32_alt_end - .Lcr4_pv32_alt)
>>> -.popsection
>>> +ALTERNATIVE_2 "jmp .Lcr4_pv32_done; .skip 2, 0x90", \
>> This changed 0xcc to 0x90 but since it is just padding following an
>> unconditional jmp so it shouldn't matter.
> Well, it was for that very reason that I had picked 0xcc originally:
> We better know if some branch mistakenly leads into that region.

Know how?  At the time you wrote this, Xen silently executed its way
through debug traps, and it took some persuading to get you to ok the
patch which at least printed a line every time we a breakpoint in
hypervisor space.

If you actually want to notice going down the wrong path, then you want
a BUG.

> I also very much object to the literal 2 passed as an argument to
> .skip above: What if the label moves out far enough that a short
> branch won't be usable anymore?

Is the commit message not enough?  a) its not going to change, because
it hasn't changed since you put the code in originally and I don't
expect it to in the future, and b) it is a temporary necessary
requirement to make the series bisectable and reviewable.  This skip is
dropped in patch 6 when the automatic padding calculations work.

~Andrew

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

Re: [Xen-devel] [RFC XEN PATCH v4 00/41] Add vNVDIMM support to HVM domains

[Jan Beulich]

>>> On 12.02.18 at 11:05,  wrote:
> If you map the NVDIMM as MMIO to Dom0 you don't need the M2P entries
> IIRC, and if it's mapped using 1GB pages it shouldn't use that much
> memory for the page tables (ie: you could just use normal RAM for the
> page tables that map the NVDIMM IMO). Of course that only applies to
> PVH/HVM.

But in order to use (part of) it in a RAM-like manner we need struct
page_info for it.

Jan


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

Re: [Xen-devel] [PATCH 3/3] pvh/dom0: whitelist PVH Dom0 ACPI tables

[Roger Pau Monné]

On Tue, Feb 13, 2018 at 02:29:08AM -0700, Jan Beulich wrote:
> >>> On 08.02.18 at 13:25,  wrote:
> > Signed-off-by: Roger Pau Monné 
> 
> A change like this should not come without description, providing a
> reason for the change. Otherwise how will someone wanting to
> understand the change in a couple of years actually be able to
> make any sense of it. This is in particular because I continue to be
> not fully convinced that white listing is appropriate in the Dom0
> case (and for the record I'm similarly unconvinced that black listing
> is the best choice, yet obviously we need to pick on of the two).

I'm sorry, I thought we agreed at the summit to convert this to
whitelisting because it was likely better to simply not expose unknown
ACPI tables to guests.

I guess the commit message could be something like:

"The following list of whitelisted APIC tables are either known to work
or don't require any resources to be mapped in either the IO or the
memory space.

This list is expected to be expanded as more testing is performed."

Thanks, Roger.

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

Re: [Xen-devel] [PATCH 4/7] x86/asm: Remove opencoded uses of altinstruction_entry

[Jan Beulich]

>>> On 12.02.18 at 13:30,  wrote:
> On Mon, Feb 12, 2018 at 11:23:04AM +, Andrew Cooper wrote:
>> diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
>> index 58f652d..bd3819a 100644
>> --- a/xen/arch/x86/x86_64/entry.S
>> +++ b/xen/arch/x86/x86_64/entry.S
>> @@ -557,23 +557,9 @@ handle_exception_saved:
>>  testb $X86_EFLAGS_IF>>8,UREGS_eflags+1(%rsp)
>>  jzexception_with_ints_disabled
>>  
>> -.Lcr4_pv32_orig:
>> -jmp   .Lcr4_pv32_done
>> -.skip (.Lcr4_pv32_alt_end - .Lcr4_pv32_alt) - (. - 
>> .Lcr4_pv32_orig), 0xcc
>> -.pushsection .altinstr_replacement, "ax"
>> -.Lcr4_pv32_alt:
>> -mov   VCPU_domain(%rbx),%rax
>> -.Lcr4_pv32_alt_end:
>> -.section .altinstructions, "a"
>> -altinstruction_entry .Lcr4_pv32_orig, .Lcr4_pv32_alt, \
>> - X86_FEATURE_XEN_SMEP, \
>> - (.Lcr4_pv32_alt_end - .Lcr4_pv32_alt), \
>> - (.Lcr4_pv32_alt_end - .Lcr4_pv32_alt)
>> -altinstruction_entry .Lcr4_pv32_orig, .Lcr4_pv32_alt, \
>> - X86_FEATURE_XEN_SMAP, \
>> - (.Lcr4_pv32_alt_end - .Lcr4_pv32_alt), \
>> - (.Lcr4_pv32_alt_end - .Lcr4_pv32_alt)
>> -.popsection
>> +ALTERNATIVE_2 "jmp .Lcr4_pv32_done; .skip 2, 0x90", \
> 
> This changed 0xcc to 0x90 but since it is just padding following an
> unconditional jmp so it shouldn't matter.

Well, it was for that very reason that I had picked 0xcc originally:
We better know if some branch mistakenly leads into that region.

I also very much object to the literal 2 passed as an argument to
.skip above: What if the label moves out far enough that a short
branch won't be usable anymore?

Jan


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

Re: [Xen-devel] [PATCH 7/7] x86: add iommu_ops to map and unmap pages, and also to flush the IOTLB

[Paul Durrant]

> -Original Message-
> From: Tian, Kevin [mailto:kevin.t...@intel.com]
> Sent: 13 February 2018 06:56
> To: Paul Durrant ; xen-devel@lists.xenproject.org
> Cc: Stefano Stabellini ; Wei Liu
> ; George Dunlap ;
> Andrew Cooper ; Ian Jackson
> ; Tim (Xen.org) ; Jan Beulich
> 
> Subject: RE: [Xen-devel] [PATCH 7/7] x86: add iommu_ops to map and
> unmap pages, and also to flush the IOTLB
> 
> > From: Paul Durrant
> > Sent: Monday, February 12, 2018 6:47 PM
> >
> > This patch adds iommu_ops to allow a domain with control_iommu
> > privilege
> > to map and unmap pages from any guest over which it has mapping
> > privilege
> > in the IOMMU.
> > These operations implicitly disable IOTLB flushing so that the caller can
> > batch operations and then explicitly flush the IOTLB using the iommu_op
> > also added by this patch.
> 
> given that last discussion is 2yrs ago and you said actual implementation
> already biased from original spec, it'd be difficult to judge whether current
> change is sufficient or just 1st step. Could you summarize what have
> been changed from last spec, and also any further tasks in your TODO list?

Kevin,

The main changes are:

- there is no op to query mapping capability... instead the hypercall will fail 
with -EACCES
- there is no longer an option to avoid reference counting map and unmap 
operations
- there are no longer separate ops for mapping local and remote pages 
(DOMID_SELF should be passed to the map op for local pages), and ops always 
deal with GFNs not MFNs
  - also I have dropped the idea of a global m2b map, so...
  - it is now going to be the responsibility of the code running in the mapping 
domain to track what it has mapped [1]
- there is no illusion that pages other 4k are supported at the moment
- the flush operation is now explicit

[1] this would be an issue if the interface becomes usable for anything other 
than dom0 as we'd also need something in Xen to release the page refs if the 
domain was forcibly destroyed, but I think the m2b was the wrong solution since 
it necessitates a full scan of *host* RAM on any domain destruction

The main item on my TODO list is to implement a new IOREQ to allow invalidation 
of specific guest pages. Think of the current 'invalidate map cache' as a 
global flush... I need a specific flush so that a decrease_reservation 
hypercall issued by a guest can instead tell emulators exactly which pages are 
being removed from guest. It is then the emulators' responsibilities to unmap 
those pages if they had them mapped (either through MMU or IOMMU) which then 
drop page refs and actually allow the pages to be recycled.

I will, of course, need to come up with more Linux code to test all this, which 
will eventually lead to kernel and user APIs to allow emulators running in dom0 
to IOMMU map guest pages.

> 
> at least just map/unmap operations definitely not meet XenGT
> requirement...
> 

What aspect of the hypercall interface does not meet XenGT's requirements? It 
would be good to know now then I can make any necessary adjustments in v2.

Cheers,

  Paul

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

Re: [Xen-devel] [PATCH 1/7] Port WARN_ON_ONCE() from Linux

[Jan Beulich]

>>> On 09.02.18 at 04:10,  wrote:
> Port WARN_ON_ONCE macro from Linux.
> 
> Signed-off-by: Sameer Goel 
> Acked-by: Julien Grall 
> ---
>  xen/arch/arm/xen.lds.S |  1 +
>  xen/arch/x86/xen.lds.S |  1 +
>  xen/include/xen/lib.h  | 13 +
>  3 files changed, 15 insertions(+)
> 
> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
> index b0390180b4..4dc7997cf0 100644
> --- a/xen/arch/arm/xen.lds.S
> +++ b/xen/arch/arm/xen.lds.S
> @@ -87,6 +87,7 @@ SECTIONS
> __end_schedulers_array = .;
> *(.data.rel)
> *(.data.rel.*)
> +   *(.data.unlikely)
> CONSTRUCTORS
>} :text
>  
> diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
> index 095298048f..353ca148ca 100644
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -240,6 +240,7 @@ SECTIONS
> *(.data)
> *(.data.rel)
> *(.data.rel.*)
> +   *(.data.unlikely)
> CONSTRUCTORS
>} :text
>  
> diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
> index 1d9771340c..697212a061 100644
> --- a/xen/include/xen/lib.h
> +++ b/xen/include/xen/lib.h
> @@ -11,6 +11,19 @@
>  #define BUG_ON(p)  do { if (unlikely(p)) BUG();  } while (0)
>  #define WARN_ON(p) do { if (unlikely(p)) WARN(); } while (0)
>  
> +#define WARN_ON_ONCE(p) \
> +({  \
> +static bool __section(".data.unlikely") __warned; \
> +int __ret_warn_once = !!(p);\

Please don't introduce new name space violations (read: no
leading underscores here; use trailing one if need be).

Jan


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

Re: [Xen-devel] [PATCH 1/7] Port WARN_ON_ONCE() from Linux

[Jan Beulich]

>>> On 09.02.18 at 11:47,  wrote:
> On Fri, Feb 09, 2018 at 10:45:25AM +, Julien Grall wrote:
>> Hi,
>> 
>> On 02/09/2018 10:29 AM, Roger Pau Monné wrote:
>> > On Thu, Feb 08, 2018 at 08:10:49PM -0700, Sameer Goel wrote:
>> > > diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
>> > > index 1d9771340c..697212a061 100644
>> > > --- a/xen/include/xen/lib.h
>> > > +++ b/xen/include/xen/lib.h
>> > > @@ -11,6 +11,19 @@
>> > >   #define BUG_ON(p)  do { if (unlikely(p)) BUG();  } while (0)
>> > >   #define WARN_ON(p) do { if (unlikely(p)) WARN(); } while (0)
>> > > +#define WARN_ON_ONCE(p) \
>> > > +({  \
>> > > +static bool __section(".data.unlikely") __warned; \
>> > > +int __ret_warn_once = !!(p);\
>> > ^ bool
>> > 
>> > > +\
>> > > +if ( unlikely(__ret_warn_once && !__warned) ) \
>> > > +{   \
>> > > +__warned = true;\
>> > > +WARN(); \
>> > > +}   \
>> > > +unlikely(__ret_warn_once);  \
>> > 
>> > Does this macro really need to return something? It seems weird to me
>> > to allow usages like: if ( WARN_ON_ONCE...
>> 
>> This construct is used in Linux (included in the driver ported):
>> 
>> if (WARN_ON_ONCE(fwspec->iommu_priv)) {
>>  master = fwspec->iommu_priv;
>>  smmu = master->smmu;
>> } else {
>> 
>> }
>> 
>> IHMO the makes the code nicer to read over:
> 
> OK, if that's intended I'm fine with it, just wanted to check.

But WARN_ON() should then be given the same property, I think.

Jan

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

Re: [Xen-devel] [PATCH 5/7] x86/alt: Support for automatic padding calculations

[Roger Pau Monné]

On Mon, Feb 12, 2018 at 11:23:05AM +, Andrew Cooper wrote:
>  .macro ALTERNATIVE oldinstr, newinstr, feature
>  .L\@_orig_s:
>  \oldinstr
>  .L\@_orig_e:
> + .skip (-((repl_len(1) - orig_len) > 0) * (repl_len(1) - orig_len)), 0x90

clang chokes on this expression, because of the negation at the
beginning and I'm also failing to see why are you adding such
negation. AFAICT using:

.skip (((repl_len(1) - orig_len) > 0) * (repl_len(1) - orig_len)), 0x90

Is correct: it adds the right padding if the alternative code is
bigger than the original one, while not adding anything is the
original code is greater than the alternative one.

The negation just turns the 1 to -1, thus converting the result of the
whole expression into a negative value.

Roger.

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

Re: [Xen-devel] [PATCH v3 2/3] x86/svm: add EFER SVME support for VGIF/VLOAD

[Jan Beulich]

>>> On 08.02.18 at 18:01,  wrote:
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -611,6 +611,12 @@ static void svm_update_guest_efer(struct vcpu *v)
>  if ( lma )
>  new_efer |= EFER_LME;
>  vmcb_set_efer(vmcb, new_efer);
> +
> +if ( !nestedhvm_enabled(v->domain) )
> +ASSERT(!(v->arch.hvm_vcpu.guest_efer & EFER_SVME));
> +
> +if ( nestedhvm_enabled(v->domain) )
> +svm_nested_features_on_efer_update(v);
>  }

Why not

if ( nestedhvm_enabled(v->domain) )
svm_nested_features_on_efer_update(v);
else
ASSERT(!(v->arch.hvm_vcpu.guest_efer & EFER_SVME));

?

Jan




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

Re: [Xen-devel] [PATCH 3/3] pvh/dom0: whitelist PVH Dom0 ACPI tables

[Jan Beulich]

>>> On 08.02.18 at 13:25,  wrote:
> Signed-off-by: Roger Pau Monné 

A change like this should not come without description, providing a
reason for the change. Otherwise how will someone wanting to
understand the change in a couple of years actually be able to
make any sense of it. This is in particular because I continue to be
not fully convinced that white listing is appropriate in the Dom0
case (and for the record I'm similarly unconvinced that black listing
is the best choice, yet obviously we need to pick on of the two).

> --- a/xen/arch/x86/hvm/dom0_build.c
> +++ b/xen/arch/x86/hvm/dom0_build.c
> @@ -789,24 +789,29 @@ static bool __init pvh_acpi_table_allowed(const char 
> *sig,
>unsigned long address,
>unsigned long size)
>  {
> -static const char __initconst banned_tables[][ACPI_NAME_SIZE] = {
> -ACPI_SIG_HPET, ACPI_SIG_SLIT, ACPI_SIG_SRAT, ACPI_SIG_MPST,
> -ACPI_SIG_PMTT, ACPI_SIG_MADT, ACPI_SIG_DMAR};
> +static const char __initconst allowed_tables[][ACPI_NAME_SIZE] = {
> +ACPI_SIG_DSDT, ACPI_SIG_FADT, ACPI_SIG_FACS, ACPI_SIG_PSDT,
> +ACPI_SIG_SSDT, ACPI_SIG_SBST, ACPI_SIG_MCFG, ACPI_SIG_SLIC,
> +ACPI_SIG_MSDM, ACPI_SIG_WDAT, ACPI_SIG_FPDT, ACPI_SIG_S3PT,
> +};
>  unsigned int i;
>  
> -for ( i = 0 ; i < ARRAY_SIZE(banned_tables); i++ )
> -if ( strncmp(sig, banned_tables[i], ACPI_NAME_SIZE) == 0 )
> -return false;
> -
> -/* Make sure table doesn't reside in a RAM region. */
> -if ( acpi_memory_banned(address, size) )
> +for ( i = 0 ; i < ARRAY_SIZE(allowed_tables); i++ )
>  {
> -printk("Skipping table %.4s because resides in a non-ACPI, 
> non-reserved region\n",
> -   sig);
> -return false;
> +if ( strncmp(sig, allowed_tables[i], ACPI_NAME_SIZE) )
> +continue;
> +
> +if ( !acpi_memory_banned(address, size) )
> +return true;
> +else

Unnecessary "else".

Jan

> +{
> +printk("Skipping table %.4s in non-ACPI non-reserved region\n",
> +   sig);
> +return false;
> +}
>  }
>  
> -return true;
> +return false;
>  }


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

Re: [Xen-devel] [PATCH 6/7] x86: add iommu_op to query reserved ranges

[Paul Durrant]

> -Original Message-
> From: Tian, Kevin [mailto:kevin.t...@intel.com]
> Sent: 13 February 2018 06:52
> To: Paul Durrant ; xen-devel@lists.xenproject.org
> Cc: Stefano Stabellini ; Wei Liu
> ; George Dunlap ;
> Andrew Cooper ; Ian Jackson
> ; Tim (Xen.org) ; Jan Beulich
> 
> Subject: RE: [Xen-devel] [PATCH 6/7] x86: add iommu_op to query reserved
> ranges
> 
> > From: Paul Durrant
> > Sent: Monday, February 12, 2018 6:47 PM
> >
> > Certain areas of memory, such as RMRRs, must be mapped 1:1
> > (i.e. BFN == MFN) through the IOMMU.
> >
> > This patch adds an iommu_op to allow these ranges to be queried.
> >
> > Signed-off-by: Paul Durrant 
> > ---
> > Cc: Jan Beulich 
> > Cc: Andrew Cooper 
> > Cc: George Dunlap 
> > Cc: Ian Jackson 
> > Cc: Konrad Rzeszutek Wilk 
> > Cc: Stefano Stabellini 
> > Cc: Tim Deegan 
> > Cc: Wei Liu 
> > ---
> >  xen/arch/x86/iommu_op.c   | 121
> > ++
> >  xen/include/public/iommu_op.h |  35 
> >  xen/include/xlat.lst  |   2 +
> >  3 files changed, 158 insertions(+)
> >
> > diff --git a/xen/arch/x86/iommu_op.c b/xen/arch/x86/iommu_op.c
> > index edd8a384b3..ac81b98b7a 100644
> > --- a/xen/arch/x86/iommu_op.c
> > +++ b/xen/arch/x86/iommu_op.c
> > @@ -22,6 +22,58 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> > +
> > +struct get_rdm_ctxt {
> > +unsigned int max_entries;
> > +unsigned int nr_entries;
> > +XEN_GUEST_HANDLE(xen_iommu_reserved_region_t) regions;
> > +};
> > +
> > +static int get_rdm(xen_pfn_t start, xen_ulong_t nr, u32 id, void *arg)
> > +{
> > +struct get_rdm_ctxt *ctxt = arg;
> > +
> > +if ( ctxt->nr_entries < ctxt->max_entries )
> > +{
> > +xen_iommu_reserved_region_t region = {
> > +.start_bfn = start,
> > +.nr_frames = nr,
> > +};
> > +
> > +if ( copy_to_guest_offset(ctxt->regions, ctxt->nr_entries, ,
> > +  1) )
> > +return -EFAULT;
> 
> RMRR entries are device specific. it's why a 'id' (i.e. sbdf) field
> is introduced for such check.

What I want here is the union of all RMRRs for all devices in the domain. I 
believe that is what the code will currently query, but I could be wrong.

> 
> > +}
> > +
> > +ctxt->nr_entries++;
> > +
> > +return 1;
> > +}
> > +
> > +static int iommuop_query_reserved(struct
> > xen_iommu_op_query_reserved *op)
> 
> I didn't get why we cannot reuse existing XENMEM_reserved_
> device_memory_map?
> 

This hypercall is not intended to be tools-only. That one is, unless I misread 
the #ifdefs.

  Paul

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

Re: [Xen-devel] [PATCH 5/7] public / x86: introduce __HYPERCALL_iommu_op

[Paul Durrant]

> -Original Message-
> From: Tian, Kevin [mailto:kevin.t...@intel.com]
> Sent: 13 February 2018 06:43
> To: Paul Durrant ; xen-devel@lists.xenproject.org
> Cc: Stefano Stabellini ; Wei Liu
> ; George Dunlap ;
> Andrew Cooper ; Ian Jackson
> ; Tim (Xen.org) ; Jan Beulich
> ; Daniel De Graaf 
> Subject: RE: [Xen-devel] [PATCH 5/7] public / x86: introduce
> __HYPERCALL_iommu_op
> 
> > From: Paul Durrant
> > Sent: Monday, February 12, 2018 6:47 PM
> >
> > This patch introduces the boilerplate for a new hypercall to allow a
> > domain to control IOMMU mappings for its own pages.
> > Whilst there is duplication of code between the native and compat entry
> > points which appears ripe for some form of combination, I think it is
> > better to maintain the separation as-is because the compat entry point
> > will necessarily gain complexity in subsequent patches.
> >
> > NOTE: This hypercall is only implemented for x86 and is currently
> >   restricted by XSM to dom0 since it could be used to cause IOMMU
> >   faults which may bring down a host.
> >
> > Signed-off-by: Paul Durrant 
> [...]
> > +
> > +
> > +static bool can_control_iommu(void)
> > +{
> > +struct domain *currd = current->domain;
> > +
> > +/*
> > + * IOMMU mappings cannot be manipulated if:
> > + * - the IOMMU is not enabled or,
> > + * - the IOMMU is passed through or,
> > + * - shared EPT configured or,
> > + * - Xen is maintaining an identity map.
> 
> "for dom0"
> 
> > + */
> > +if ( !iommu_enabled || iommu_passthrough ||
> > + iommu_use_hap_pt(currd) || need_iommu(currd) )
> 
> I guess it's clearer to directly check iommu_dom0_strict here

Well, the problem with that is that it totally ties this interface to dom0. 
Whilst, in practice, that is the case at the moment (because of the xsm check) 
I do want to leave the potential to allow other PV domains to control their 
IOMMU mappings, if that make sense in future.

  Paul

> 
> > +return false;
> > +
> > +return true;
> > +}
> 

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

[Xen-devel] [linux-linus bisection] complete test-amd64-i386-xl-qemuu-ovmf-amd64

[osstest service owner]

branch xen-unstable
xenbranch xen-unstable
job test-amd64-i386-xl-qemuu-ovmf-amd64
testid xen-boot

Tree: linux git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git

*** Found and reproduced problem changeset ***

  Bug is in tree:  linux 
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
  Bug introduced:  7928b2cbe55b2a410a0f5c1f154610059c57b1b2
  Bug not present: d8a5b80568a9cb66810e75b182018e9edb68e8ff
  Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/119063/


  (Revision log too long, omitted.)


For bisection revision-tuple graph see:
   
http://logs.test-lab.xenproject.org/osstest/results/bisect/linux-linus/test-amd64-i386-xl-qemuu-ovmf-amd64.xen-boot.html
Revision IDs in each graph node refer, respectively, to the Trees above.


Running cs-bisection-step 
--graph-out=/home/logs/results/bisect/linux-linus/test-amd64-i386-xl-qemuu-ovmf-amd64.xen-boot
 --summary-out=tmp/119063.bisection-summary --basis-template=118324 
--blessings=real,real-bisect linux-linus test-amd64-i386-xl-qemuu-ovmf-amd64 
xen-boot
Searching for failure / basis pass:
 118968 fail [host=huxelrebe0] / 118629 [host=rimava0] 118598 [host=baroque0] 
118586 [host=italia0] 118576 [host=huxelrebe1] 118566 [host=elbling1] 118556 
[host=fiano0] 118538 [host=baroque1] 118501 [host=elbling0] 118464 
[host=chardonnay1] 118445 [host=pinot0] 118428 ok.
Failure / basis pass flights: 118968 / 118428
(tree with no url: minios)
(tree with no url: ovmf)
(tree with no url: seabios)
Tree: linux git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git
Latest 7928b2cbe55b2a410a0f5c1f154610059c57b1b2 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
c8ea0457495342c417c3dc033bba25148b279f60 
2b033e396f4fa0981bae1213cdacd15775655a97 
c93014ad3aa6aa88dfa5e96f66e8adb561483b8d
Basis pass d8a5b80568a9cb66810e75b182018e9edb68e8ff 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
c8ea0457495342c417c3dc033bba25148b279f60 
2b033e396f4fa0981bae1213cdacd15775655a97 
e871e80c38547d9faefc6604532ba3e985e65873
Generating revisions with ./adhoc-revtuple-generator  
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git#d8a5b80568a9cb66810e75b182018e9edb68e8ff-7928b2cbe55b2a410a0f5c1f154610059c57b1b2
 
git://xenbits.xen.org/osstest/linux-firmware.git#c530a75c1e6a472b0eb9558310b518f0dfcd8860-c530a75c1e6a472b0eb9558310b518f0dfcd8860
 
git://xenbits.xen.org/qemu-xen-traditional.git#c8ea0457495342c417c3dc033bba25148b279f60-c8ea0457495342c417c3dc033bba25148b279f60
 
git://xenbits.xen.org/qemu-xen.git#2b033e396f4fa0981bae1213cdacd15775655a97-2b033e396f4fa0981bae1213cdacd15775655a97
 
git://xenbits.xen.org/xen.git#e871e80c38547d9faefc6604532ba3e985e65873-c93014ad3aa6aa88dfa5e96f66e8adb561483b8d
adhoc-revtuple-generator: tree discontiguous: linux-2.6
Loaded 1002 nodes in revision graph
Searching for test results:
 118112 [host=chardonnay1]
 118215 [host=rimava1]
 118250 [host=baroque1]
 118276 [host=rimava0]
 118283 [host=pinot1]
 118324 [host=chardonnay0]
 118445 [host=pinot0]
 118362 [host=italia1]
 118401 [host=italia0]
 118428 pass d8a5b80568a9cb66810e75b182018e9edb68e8ff 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
c8ea0457495342c417c3dc033bba25148b279f60 
2b033e396f4fa0981bae1213cdacd15775655a97 
e871e80c38547d9faefc6604532ba3e985e65873
 118464 [host=chardonnay1]
 118538 [host=baroque1]
 118501 [host=elbling0]
 118556 [host=fiano0]
 118566 [host=elbling1]
 118576 [host=huxelrebe1]
 118586 [host=italia0]
 118629 [host=rimava0]
 118598 [host=baroque0]
 118638 fail irrelevant
 118672 fail irrelevant
 118775 fail irrelevant
 118893 fail irrelevant
 119058 pass d8a5b80568a9cb66810e75b182018e9edb68e8ff 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
c8ea0457495342c417c3dc033bba25148b279f60 
2b033e396f4fa0981bae1213cdacd15775655a97 
c93014ad3aa6aa88dfa5e96f66e8adb561483b8d
 119047 fail 7928b2cbe55b2a410a0f5c1f154610059c57b1b2 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
c8ea0457495342c417c3dc033bba25148b279f60 
2b033e396f4fa0981bae1213cdacd15775655a97 
c93014ad3aa6aa88dfa5e96f66e8adb561483b8d
 119004 fail 7928b2cbe55b2a410a0f5c1f154610059c57b1b2 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
c8ea0457495342c417c3dc033bba25148b279f60 
2b033e396f4fa0981bae1213cdacd15775655a97 
c93014ad3aa6aa88dfa5e96f66e8adb561483b8d
 119012 pass d8a5b80568a9cb66810e75b182018e9edb68e8ff 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
c8ea0457495342c417c3dc033bba25148b279f60 
2b033e396f4fa0981bae1213cdacd15775655a97 
4dcfd7d1436c77ee92081a36cf63f569dc4ef725
 119035 pass 

Re: [Xen-devel] [PATCH 0/7] paravirtual IOMMU interface

[Paul Durrant]

> -Original Message-
> From: Tian, Kevin [mailto:kevin.t...@intel.com]
> Sent: 13 February 2018 06:21
> To: Paul Durrant ; xen-devel@lists.xenproject.org
> Cc: Andrew Cooper ; Daniel De Graaf
> ; George Dunlap ;
> Ian Jackson ; Jan Beulich ;
> Julien Grall ; Nakajima, Jun
> ; Konrad Rzeszutek Wilk
> ; Stefano Stabellini ;
> Suravee Suthikulpanit ; Tim (Xen.org)
> ; Wei Liu 
> Subject: RE: [PATCH 0/7] paravirtual IOMMU interface
> 
> > From: Paul Durrant [mailto:paul.durr...@citrix.com]
> > Sent: Monday, February 12, 2018 6:47 PM
> >
> > The idea of a paravirtual IOMMU interface was last discussed on xen-devel
> > more than two years ago and narrowed down on a draft specification [1].
> > There was also an RFC patch series posted with an implementation,
> > however
> > this was never followed through.
> >
> > In this patch series I have tried to simplify the interface and therefore
> > have moved away from the draft specification.
> 
> bear sending out an updated spec?
> 

I'll have to write one, but I agree it is probably worthwhile for the record. 
The intention is the same as it was when the old spec. was written but I hope 
this implementation is less complex (though it may not yet be fully complete).
In the meantime I hope each patch is sufficiently small to be reasonably 
self-explanatory.

Cheers,

  Paul

> >
> > Patches #1 - #3 in the series introduce 'bus frame numbers' into Xen (frame
> > numbers relating to the IOMMU rather than the MMU). The modifications
> > are
> > in common code and so affect ARM as well as x86.
> >
> > Patch #4 adds a pre-requisite method in iommu_ops and an
> > implementation
> > for VT-d. I have not done an implmentation for AMD IOMMUs as my test
> > hard-
> > ware is Intel based, but one may be added in future.
> >
> > Patches #5 - #7 introduce the new 'iommu_op' hypercall with sub-
> > operations
> > to query ranges reserved in the IOMMU, map and unmap pages, and flush
> > the
> > IOTLB.
> >
> > For testing purposes, I have implemented patches to a Linux PV dom0 to
> > set
> > up a 1:1 BFN:GFN mapping and use normal swiotlb dma operations rather
> > then xen-swiotlb.
> >
> > [1] https://lists.xenproject.org/archives/html/xen-devel/2016-
> > 02/msg01428.html
> >
> > Paul Durrant (7):
> >   iommu: introduce the concept of BFN...
> >   iommu: make use of type-safe BFN and MFN in exported functions
> >   iommu: push use of type-safe BFN and MFN into iommu_ops
> >   vtd: add lookup_page method to iommu_ops
> >   public / x86: introduce __HYPERCALL_iommu_op
> >   x86: add iommu_op to query reserved ranges
> >   x86: add iommu_ops to map and unmap pages, and also to flush the
> > IOTLB
> >
> >  tools/flask/policy/modules/xen.if |   1 +
> >  xen/arch/arm/p2m.c|   3 +-
> >  xen/arch/x86/Makefile |   1 +
> >  xen/arch/x86/hvm/hypercall.c  |   1 +
> >  xen/arch/x86/hypercall.c  |   1 +
> >  xen/arch/x86/iommu_op.c   | 476
> > ++
> >  xen/arch/x86/mm.c |   7 +-
> >  xen/arch/x86/mm/p2m-ept.c |   8 +-
> >  xen/arch/x86/mm/p2m-pt.c  |   8 +-
> >  xen/arch/x86/mm/p2m.c |  15 +-
> >  xen/arch/x86/pv/hypercall.c   |   1 +
> >  xen/arch/x86/x86_64/mm.c  |   5 +-
> >  xen/common/grant_table.c  |  10 +-
> >  xen/common/memory.c   |   4 +-
> >  xen/drivers/passthrough/amd/iommu_cmd.c   |  18 +-
> >  xen/drivers/passthrough/amd/iommu_map.c   |  85 ++---
> >  xen/drivers/passthrough/amd/pci_amd_iommu.c   |   4 +-
> >  xen/drivers/passthrough/arm/smmu.c|  22 +-
> >  xen/drivers/passthrough/iommu.c   |  28 +-
> >  xen/drivers/passthrough/vtd/iommu.c   |  76 +++-
> >  xen/drivers/passthrough/vtd/iommu.h   |   2 +
> >  xen/drivers/passthrough/vtd/x86/vtd.c |   3 +-
> >  xen/drivers/passthrough/x86/iommu.c   |   2 +-
> >  xen/include/Makefile  |   2 +
> >  xen/include/asm-x86/hvm/svm/amd-iommu-proto.h |   8 +-
> >  xen/include/public/iommu_op.h | 127 +++
> >  xen/include/public/xen.h  |   1 +
> >  xen/include/xen/hypercall.h   |  12 +
> >  xen/include/xen/iommu.h   |  42 ++-
> >  xen/include/xlat.lst  |   5 +
> >  xen/include/xsm/dummy.h   |   6 +
> >  xen/include/xsm/xsm.h |   6 +
> >  xen/xsm/dummy.c   |   1 +
> >  

Re: [Xen-devel] [PATCH v3 2/2] x86/setup: remap Xen image up to PFN_DOWN(__pa(_end))

[Jan Beulich]

>>> On 08.02.18 at 14:46,  wrote:
> Sorry for late reply but I was busy with other stuff.
> 
> On Fri, Jan 19, 2018 at 08:27:46AM -0700, Jan Beulich wrote:
>> >>> On 10.01.18 at 14:05,  wrote:
>> > Current limit, PFN_DOWN(xen_phys_start), introduced by commit b280442
>> > (x86: make Xen early boot code relocatable) is not reliable. Potentially
>> > its value may fall below PFN_DOWN(__pa(_end)) and then part of Xen image
>> > may not be mapped after relocation. This will not happen in current code
>> > thanks to "x86/setup: do not relocate over current Xen image placement"
>> > patch. Though this safety measure may save a lot of debugging time when
>> > somebody decide to relax existing relocation restrictions one day.
>>
>> I've gone back through the v2 discussion, and I continue to fail to
>> see what is being fixed here, even if just theoretically. It is bad
> 
> OK, let's give an example. I assume that there is no patch 1 and Xen can
> relocate itself even it was initially relocated by the bootloader. So,
> let's assume that the bootloader loaded Xen image at 0x8020
> (xen_phys_start == 0x8000) and its size is 0x70 (7 MiB).
> The RAM region ends at 0x80D0 and there is no RAM above that
> address. At some point Xen realizes that it can relocate itself
> to 0x8060 (xen_phys_start == 0x8040). So, it does and then
> remaps itself. And here is the problem. Currently existing code
> will remap only Xen image up to 0x803f. Everything above will
> no be remapped. So, that is why I suggested this patch.
> 
>> enough that the description here isn't clarifying this and I need to
>> go back to the earlier discussion, but it's even worse if even that
>> earlier discussion didn't really help. My conclusion is that you're
> 
> Sorry about that.
> 
>> talking about a case where old and positions of Xen overlap, a
>> case which I thought patch 1 eliminates.
> 
> It does not eliminate the issue described above. It just hides it.

Well, no, I disagree - it makes an overlap impossible afaict,
which is more that just hiding the problem. Anyway - I'm not
going to object to the change provided it comes with a clear
description of what _existing_ issue (even if just a theoretical
one) is being fixed _with the currently present code in mind_
(i.e. in particular including your patch 1).

Jan


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

Re: [Xen-devel] [PATCH RFC v2 10/12] x86: allocate per-vcpu stacks for interrupt entries

[Jan Beulich]

>>> On 09.02.18 at 13:35,  wrote:
> On 30/01/18 16:40, Jan Beulich wrote:
> On 22.01.18 at 13:32,  wrote:
>>> @@ -37,10 +52,24 @@ struct vcpu;
>>>  
>>>  struct cpu_info {
>>>  struct cpu_user_regs guest_cpu_user_regs;
>>> -unsigned int processor_id;
>>> -struct vcpu *current_vcpu;
>>> -unsigned long per_cpu_offset;
>>> -unsigned long cr4;
>>> +union {
>>> +/* per physical cpu mapping */
>>> +struct {
>>> +struct vcpu *current_vcpu;
>>> +unsigned long per_cpu_offset;
>>> +unsigned long cr4;
>>> +};
>>> +/* per vcpu mapping (xpti) */
>>> +struct {
>>> +unsigned long pad1;
>>> +unsigned long pad2;
>>> +unsigned long stack_bottom_cpu;
>>> +};
>> 
>> In order to avoid accidental use in the wrong context as much as
>> possible, I think you want to name both structures.
> 
> I'd like to leave it as is in order to make a possible backport much
> more easier.

Well, I can see why you would want the pre-existing fields left
without structure field name, but the new (vcpu) ones? And
even the pre-existing (pcpu) ones should gain a name, just
perhaps in a patch late in the series, which then wouldn't be
backported.

Jan


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

Re: [Xen-devel] [PATCH] x86/nmi: lower initial watchdog frequency to avoid boot hangs

[Jan Beulich]

>>> On 08.02.18 at 13:18,  wrote:
> We switch the NMI frequency to ~2Hz after the calibration, but that is
> after having run the BSP at 100Hz for a long period of time, and the APs
> at this rate for a short while.  Irrespective of the exact fix here, it
> is simply not a good idea to be running with this NMI frequency, other
> than possibly during the immediate calibration logic.

And I've previously agreed with this general statement. It's
just that the so far suggested workaround doesn't really do
what you say (and I'm pretty determined that the "possibly"
above should be removed).

Jan


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

[Xen-devel] [linux-linus test] 119010: regressions - trouble: broken/fail/pass

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemuu-ovmf-amd64 broken
 test-amd64-i386-xl-xsm7 xen-boot fail REGR. vs. 118324
 test-amd64-i386-libvirt   7 xen-boot fail REGR. vs. 118324
 test-amd64-i386-xl-qemuu-ovmf-amd64  7 xen-boot  fail REGR. vs. 118324
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 
118324
 test-amd64-i386-xl-qemuu-win10-i386  7 xen-boot  fail REGR. vs. 118324
 test-amd64-i386-freebsd10-amd64  7 xen-boot  fail REGR. vs. 118324
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm  7 xen-boot fail REGR. vs. 118324
 test-amd64-i386-xl-qemut-win7-amd64  7 xen-boot  fail REGR. vs. 118324
 test-amd64-i386-qemuu-rhel6hvm-amd  7 xen-boot   fail REGR. vs. 118324
 test-amd64-i386-xl-raw7 xen-boot fail REGR. vs. 118324
 test-amd64-i386-xl-qemut-debianhvm-amd64  7 xen-boot fail REGR. vs. 118324
 test-amd64-i386-qemut-rhel6hvm-amd  7 xen-boot   fail REGR. vs. 118324
 test-amd64-amd64-xl-pvhv2-intel 12 guest-start   fail REGR. vs. 118324
 test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail REGR. vs. 118324
 test-amd64-i386-examine   8 reboot   fail REGR. vs. 118324
 test-amd64-i386-xl-qemut-ws16-amd64  7 xen-boot  fail REGR. vs. 118324
 test-amd64-i386-qemuu-rhel6hvm-intel  7 xen-boot fail REGR. vs. 118324
 test-amd64-i386-pair 10 xen-boot/src_hostfail REGR. vs. 118324
 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 118324
 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail REGR. vs. 118324
 test-amd64-i386-pair 11 xen-boot/dst_hostfail REGR. vs. 118324
 test-amd64-i386-qemut-rhel6hvm-intel  7 xen-boot fail REGR. vs. 118324
 test-amd64-i386-xl-qemut-win10-i386  7 xen-boot  fail REGR. vs. 118324
 test-amd64-i386-rumprun-i386  7 xen-boot fail REGR. vs. 118324
 test-amd64-i386-xl-qemuu-win7-amd64  7 xen-boot  fail REGR. vs. 118324
 test-amd64-i386-freebsd10-i386  7 xen-boot   fail REGR. vs. 118324
 test-amd64-i386-xl-qemuu-debianhvm-amd64  7 xen-boot fail REGR. vs. 118324
 test-amd64-i386-libvirt-xsm   7 xen-boot fail REGR. vs. 118324
 test-amd64-i386-xl7 xen-boot fail REGR. vs. 118324
 test-amd64-i386-xl-qemuu-ws16-amd64  7 xen-boot  fail REGR. vs. 118324
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 
118324
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm  7 xen-boot fail REGR. vs. 118324

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-qemuu-ovmf-amd64  4 host-install(4)  broken pass in 118968
 test-armhf-armhf-xl-rtds  6 xen-installfail pass in 118968

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-rtds13 migrate-support-check fail in 118968 never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-check fail in 118968 never pass
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail  like 118324
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 118324
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 118324
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 118324
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 118324
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 118324
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 118324
 test-amd64-amd64-libvirt-xsm 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-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14