[Xen-devel] [PATCH] xen/sched: remove wrong assertions in csched2_free_pdata()

2019-11-07 Thread Juergen Gross
The assertions in csched2_free_pdata() are wrong as in case it is
called by schedule_cpu_add() after a failure of sched_alloc_udata()
the init pdata function won't have been called.

So just remove the (wrong) comment and ASSERT() statements.

While at it remove the wrong comment in csched2_deinit_pdata(), too.

Signed-off-by: Juergen Gross 
---
 xen/common/sched_credit2.c | 16 
 1 file changed, 16 deletions(-)

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index af58ee161d..a995ff838f 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -3914,10 +3914,6 @@ csched2_deinit_pdata(const struct scheduler *ops, void 
*pcpu, int cpu)
 
 write_lock_irqsave(>lock, flags);
 
-/*
- * alloc_pdata is not implemented, so pcpu must be NULL. On the other
- * hand, init_pdata must have been called for this pCPU.
- */
 /*
  * Scheduler specific data for this pCPU must still be there and and be
  * valid. In fact, if we are here:
@@ -3969,18 +3965,6 @@ csched2_deinit_pdata(const struct scheduler *ops, void 
*pcpu, int cpu)
 static void
 csched2_free_pdata(const struct scheduler *ops, void *pcpu, int cpu)
 {
-struct csched2_pcpu *spc = pcpu;
-
-/*
- * pcpu either points to a valid struct csched2_pcpu, or is NULL (if
- * CPU bringup failed, and we're beeing called from CPU_UP_CANCELLED).
- * xfree() does not really mind, but we want to be sure that either
- * init_pdata has never been called, or deinit_pdata has been called
- * already.
- */
-ASSERT(!pcpu || spc->runq_id == -1);
-ASSERT(!cpumask_test_cpu(cpu, _priv(ops)->initialized));
-
 xfree(pcpu);
 }
 
-- 
2.16.4


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

Re: [Xen-devel] [PATCH v1 04/10] vfio/type1: Prepare is_invalid_reserved_pfn() for PG_reserved changes

2019-11-07 Thread David Hildenbrand

On 08.11.19 06:09, Dan Williams wrote:

On Thu, Nov 7, 2019 at 2:07 PM David Hildenbrand  wrote:


On 07.11.19 19:22, David Hildenbrand wrote:




Am 07.11.2019 um 16:40 schrieb Dan Williams :

On Thu, Oct 24, 2019 at 5:12 AM David Hildenbrand  wrote:


Right now, ZONE_DEVICE memory is always set PG_reserved. We want to
change that.

KVM has this weird use case that you can map anything from /dev/mem
into the guest. pfn_valid() is not a reliable check whether the memmap
was initialized and can be touched. pfn_to_online_page() makes sure
that we have an initialized memmap (and don't have ZONE_DEVICE memory).

Rewrite is_invalid_reserved_pfn() similar to kvm_is_reserved_pfn() to make
sure the function produces the same result once we stop setting ZONE_DEVICE
pages PG_reserved.

Cc: Alex Williamson 
Cc: Cornelia Huck 
Signed-off-by: David Hildenbrand 
---
drivers/vfio/vfio_iommu_type1.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 2ada8e6cdb88..f8ce8c408ba8 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -299,9 +299,15 @@ static int vfio_lock_acct(struct vfio_dma *dma, long 
npage, bool async)
   */
static bool is_invalid_reserved_pfn(unsigned long pfn)
{
-   if (pfn_valid(pfn))
-   return PageReserved(pfn_to_page(pfn));
+   struct page *page = pfn_to_online_page(pfn);


Ugh, I just realized this is not a safe conversion until
pfn_to_online_page() is moved over to subsection granularity. As it
stands it will return true for any ZONE_DEVICE pages that share a
section with boot memory.


That should not happen right now and I commented back when you introduced 
subsection support that I don’t want to have ZONE_DEVICE mixed with online 
pages in a section. Having memory block devices that partially span ZONE_DEVICE 
would be ... really weird. With something like pfn_active() - as discussed - we 
could at least make this check work - but I am not sure if we really want to go 
down that path. In the worst case, some MB of RAM are lost ... I guess this 
needs more thought.



I just realized the "boot memory" part. Is that a real thing? IOW, can
we have ZONE_DEVICE falling into a memory block (with holes)? I somewhat
have doubts that this would work ...


One of the real world failure cases that started the subsection effect
is that Persistent Memory collides with System RAM on a 64MB boundary
on shipping platforms. System RAM ends on a 64MB boundary and due to a
lack of memory controller resources PMEM is mapped contiguously at the
end of that boundary. Some more details in the subsection cover letter
/ changelogs [1] [2]. It's not sufficient to just lose some memory,
that's the broken implementation that lead to the subsection work
because the lost memory may change from one boot to the next and
software can't reliably inject a padding that conforms to the x86
128MB section constraint.


Thanks, I thought it was mostly for weird alignment where other parts of 
the section are basically "holes" and not memory.


Yes, it is a real bug that ZONE_DEVICE pages fall into sections that are 
marked SECTION_IS_ONLINE.




Suffice to say I think we need your pfn_active() to get subsection
granularity pfn_to_online_page() before PageReserved() can be removed.


I agree that we have to fix this. I don't like ZONE_DEVICE pages falling 
into memory device blocks (e.g., cannot get offlined), but I guess that 
train is gone :) As long as it's not for memory hotplug, I can most 
probably live with this.


Also, I'd like to get Michals opinion on this and the pfn_active() 
approach, but I can understand he's busy.


This patch set can wait, I won't be working next week besides 
reading/writing mails either way.


Is anybody looking into the pfn_active() thingy?



[1]: 
https://lore.kernel.org/linux-mm/156092349300.979959.17603710711957735135.st...@dwillia2-desk3.amr.corp.intel.com/
[2]: 
https://lore.kernel.org/linux-mm/156092354368.979959.6232443923440952359.st...@dwillia2-desk3.amr.corp.intel.com/




--

Thanks,

David / dhildenb


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

[Xen-devel] [xen-4.10-testing test] 143854: regressions - trouble: fail/pass/starved

2019-11-07 Thread osstest service owner
flight 143854 xen-4.10-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/143854/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qcow2   19 guest-start/debian.repeat fail REGR. vs. 139091
 test-amd64-amd64-libvirt-vhd 17 guest-start/debian.repeat fail REGR. vs. 139091
 test-amd64-i386-xl-raw  19 guest-start/debian.repeat fail REGR. vs. 139091
 test-armhf-armhf-libvirt-raw 15 guest-start/debian.repeat fail REGR. vs. 139091
 test-armhf-armhf-xl-vhd 15 guest-start/debian.repeat fail REGR. vs. 139091

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

[Xen-devel] [PATCH] xen/sched: fix a potential issue with core scheduling

2019-11-07 Thread Juergen Gross
cpupool_online_cpumask() is used by credit and rt scheduler. It returns
all the cpus of a cpupool or all online cpus in case no cpupool is
specified.

The "no cpupool" case can be dropped, as no scheduler other than the
init scheduler will ever work on cpus not associated with any cpupool.

As the individual schedulers should only ever work on scheduling
resources instead of individual cpus, their cpupool_online_cpumask()
use should be replaced by cpupool->res_valid.

Note that only with core scheduling active this might result in
potential problems, as with cpu scheduling both masks are identical.

Signed-off-by: Juergen Gross 
---
 xen/common/sched_credit.c  | 3 +--
 xen/common/sched_rt.c  | 4 ++--
 xen/include/xen/sched-if.h | 3 ---
 3 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index fbffcf3996..645cdc5e9a 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -1684,12 +1684,11 @@ csched_load_balance(struct csched_private *prv, int cpu,
 struct cpupool *c = get_sched_res(cpu)->cpupool;
 struct csched_unit *speer;
 cpumask_t workers;
-cpumask_t *online;
+cpumask_t *online = c->res_valid;
 int peer_cpu, first_cpu, peer_node, bstep;
 int node = cpu_to_node(cpu);
 
 BUG_ON(get_sched_res(cpu) != snext->unit->res);
-online = cpupool_online_cpumask(c);
 
 /*
  * If this CPU is going offline, or is not (yet) part of any cpupool
diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c
index 6e93e50acb..b2b29481f3 100644
--- a/xen/common/sched_rt.c
+++ b/xen/common/sched_rt.c
@@ -774,8 +774,8 @@ rt_deinit_pdata(const struct scheduler *ops, void *pcpu, 
int cpu)
 
 if ( prv->repl_timer.cpu == cpu )
 {
-struct cpupool *c = get_sched_res(cpu)->cpupool;
-unsigned int new_cpu = cpumask_cycle(cpu, cpupool_online_cpumask(c));
+cpumask_t *online = get_sched_res(cpu)->cpupool->res_valid;
+unsigned int new_cpu = cpumask_cycle(cpu, online);
 
 /*
  * Make sure the timer run on one of the cpus that are still available
diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h
index 29715652bc..b0ac54e63d 100644
--- a/xen/include/xen/sched-if.h
+++ b/xen/include/xen/sched-if.h
@@ -545,9 +545,6 @@ struct cpupool
 enum sched_gran  gran;
 };
 
-#define cpupool_online_cpumask(_pool) \
-(((_pool) == NULL) ? _online_map : (_pool)->cpu_valid)
-
 static inline cpumask_t *cpupool_domain_master_cpumask(const struct domain *d)
 {
 /*
-- 
2.16.4


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

Re: [Xen-devel] [BUGFIX PATCH for-4.13] sched: fix dom0less boot with the null scheduler

2019-11-07 Thread Jürgen Groß

On 06.11.19 16:58, Dario Faggioli wrote:

In a dom0less configuration, if the null scheduler is used, the system
may fail to boot, because the loop in null_unit_wake() never exits.

Bisection showed that this behavior occurs since commit d545f1d6 ("xen:
sched: deal with vCPUs being or becoming online or offline") but the
real problem is that, in this case, pick_res() always return the same
CPU.

Fix this by only deal with the simple case, i.e., the vCPU that is
coming online can be assigned to a sched. resource right away, in
null_unit_wake().

If it can't, just add it to the waitqueue, and we will deal with it in
null_schedule(), being careful about not racing with vcpu_wake().

Reported-by: Stefano Stabellini 
Signed-off-by: Dario Faggioli 
Tested-by: Stefano Stabellini 


Release-acked-by: Juergen Gross 


Juergen

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

Re: [Xen-devel] [PATCH v2 02/15] mm/mmu_notifier: add an interval tree notifier

2019-11-07 Thread Christoph Hellwig
On Thu, Nov 07, 2019 at 08:06:08PM +, Jason Gunthorpe wrote:
> > 
> > enum mmu_range_notifier_event {
> > MMU_NOTIFY_RELEASE,
> > };
> > 
> > ...assuming that we stay with "mmu_range_notifier" as a core name for this 
> > whole thing.
> > 
> > Also, it is best moved down to be next to the new MNR structs, so that all 
> > the
> > MNR stuff is in one group.
> 
> I agree with Jerome, this enum is part of the 'struct
> mmu_notifier_range' (ie the description of the invalidation) and it
> doesn't really matter that only these new notifiers can be called with
> this type, it is still part of the mmu_notifier_range.
> 
> The comment already says it only applies to the mmu_range_notifier
> scheme..

In fact the enum is entirely unused.  We might as well just kill it off
entirely.

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

Re: [Xen-devel] [XEN PATCH for-4.13] libxl: Fix setting vncpasswd to empty string

2019-11-07 Thread Jürgen Groß

On 04.11.19 16:30, Anthony PERARD wrote:

Before 93dcc22, error from setting the vnc password to an empty
string, when QEMU wasn't expected a password, never prevented the creation
of a guest, and only logged an error message.

Reported-by: Roger Pau Monné 
Fixes: 93dcc22fe798c9fa5ce117f1ed6db0d8bd779020
Signed-off-by: Anthony PERARD 


Release-acked-by: Juergen Gross 


Juergen

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

Re: [Xen-devel] [PATCH for-4.13] tools: Fix local variable block

2019-11-07 Thread Jürgen Groß

On 01.11.19 20:13, Andrew Cooper wrote:

c-indent-level isn't considered a safe variable, and "solaris" isn't a
recognised C style.  Both cause prompts when opening the files.

Fix all blocks up per CODING_STYLE

Signed-off-by: Andrew Cooper 


Release-acked-by: Juergen Gross 


Juergen

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

Re: [Xen-devel] [XEN PATCH for-4.13] libxl_pci: Don't hold QMP connection while waiting

2019-11-07 Thread Jürgen Groß

On 31.10.19 13:17, Anthony PERARD wrote:

After sending the 'device_del' command for a PCI passthrough device,
we wait until QEMU has effectively deleted the device, this involves
executing more QMP commands. While waiting, libxl hold the connection.

It isn't necessary to hold the connection and it prevents others from
making progress, so this patch releases the QMP connection.

For background:
 e.g., when a guest is created with several pci passthrough
 attached, on `xl destroy` all the devices needs to be detach, and
 this is usually what happens:
- 'device_del' called for the 1st pci device
- 'query-pci' checking if pci still there, it is
- wait 1s
- 'query-pci' checking again, and it's gone
-> now the same can be done for the second pci device, so
plenty of waiting on others when pci detach can be done in
parallel.

 On shutdown, libxl usually keeps waiting because QEMU never
 releases the device because the guest kernel never responds QEMU's
 unplug queries. So detaching of the 1st device waits until a
 timeout stops it, and since the same timeout is setup at the same
 time for the other devices to detach, the 'device_del' command is
 never sent for those.

Signed-off-by: Anthony PERARD 


Release-acked-by: Juergen Gross 


Juergen


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

Re: [Xen-devel] [XEN PATCH for-4.13 v2 0/6] Fix: libxl workaround, multiple connection to single QMP socket

2019-11-07 Thread Jürgen Groß

On 30.10.19 19:06, Anthony PERARD wrote:

Patch series available in this git branch:
https://xenbits.xen.org/git-http/people/aperard/xen-unstable.git 
br.fix-ev_qmp-multi-connect-v2

Hi,

QEMU's QMP socket doesn't allow multiple concurrent connection. Also, it
listen() on the socket with a `backlog' of only 1. On Linux at least, once that
backlog is filled connect() will return EAGAIN if the socket fd is
non-blocking. libxl may attempt many concurrent connect() attempt if for
example a guest is started with several PCI passthrough devices, and a
connect() failure lead to a failure to start the guest.

Since we can't change the listen()'s `backlog' that QEMU use, we need other
ways to workaround the issue. This patch series introduce a lock to acquire
before attempting to connect() to the QMP socket. Since the lock might be held
for to long, the series also introduce a way to cancel the acquisition of the
lock, this means killing the process that tries to get the lock.

See thread[1] for discussed alternative.
[1] https://lists.xenproject.org/archives/html/xen-devel/2019-10/msg01815.html

Cheers,

Anthony PERARD (6):
   libxl: Introduce libxl__ev_child_kill_deregister
   libxl: Move libxl__ev_devlock declaration
   libxl: Rename ev_devlock to ev_slowlock
   libxl: Introduce libxl__ev_slowlock_dispose
   libxl: libxl__ev_qmp_send now takes an egc
   libxl_qmp: Have a lock for QMP socket access

  tools/libxl/libxl_disk.c|  16 ++--
  tools/libxl/libxl_dm.c  |   8 +-
  tools/libxl/libxl_dom_save.c|   2 +-
  tools/libxl/libxl_dom_suspend.c |   2 +-
  tools/libxl/libxl_domain.c  |  18 ++---
  tools/libxl/libxl_event.c   |   6 +-
  tools/libxl/libxl_fork.c|  48 
  tools/libxl/libxl_internal.c|  41 +++---
  tools/libxl/libxl_internal.h| 130 +++-
  tools/libxl/libxl_pci.c |   8 +-
  tools/libxl/libxl_qmp.c | 119 -
  tools/libxl/libxl_usb.c |  28 ---
  12 files changed, 301 insertions(+), 125 deletions(-)



For the series:

Release-acked-by: Juergen Gross 


Juergen

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

Re: [Xen-devel] [XEN PATCH for-4.13] tools/configure: Honour XEN_COMPILE_ARCH and _TARGET_ for shim

2019-11-07 Thread Jürgen Groß

On 29.10.19 18:57, Ian Jackson wrote:

The pvshim can only be built 64-bit because the hypervisor is only
64-bit nowadays.  The hypervisor build supports XEN_COMPILE_ARCH and
XEN_TARGET_ARCH which override the information from uname.  The pvshim
build runs out of the tools/ directory but calls the hypervisor build
system.

If one runs in a Linux 32-bit userland with a 64-bit kernel, one used
to be able to set XEN_COMPILE_ARCH.  But nowadays this does not work.
configure sees the target cpu as 64-bit and tries to build pvshim.
The build prints
   echo "*** Xen x86/32 target no longer supported!"
and doesn't build anything.  Then the subsequent Makefiles try to
install the non-built pieces.

Fix this anomaly by causing configure to honour the Xen hypervisor way
of setting the target architecture.

In principle this user behaviour is not handled quite right, because
configure will still see 64-bit and so all the autoconf-based
architecture testing will see 64-bit rather than 32-bit x86.  But the
tools are in fact generally quite portable: this particular location
in configure{.ac,} is the only place in tools/ where 64-bit x86 is
treated differently from 32-bit x86, so the fix is sufficient and
correct for this use case.

It remains the case that XEN_COMPILE_ARCH or XEN_TARGET_ARCH to a
non-x86 architecture, when configure thinks things are x86, or vice
versa, will not work right.

I have rerun autogen.sh, so this patch contains the fix to configure
as well as the source fix to configure.ac.

Signed-off-by: Ian Jackson 
CC: Jürgen Groß 


Release-acked-by: Juergen Gross 


Juergen

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

Re: [Xen-devel] [PATCH v1 04/10] vfio/type1: Prepare is_invalid_reserved_pfn() for PG_reserved changes

2019-11-07 Thread David Hildenbrand


> Am 07.11.2019 um 16:40 schrieb Dan Williams :
> 
> On Thu, Oct 24, 2019 at 5:12 AM David Hildenbrand  wrote:
>> 
>> Right now, ZONE_DEVICE memory is always set PG_reserved. We want to
>> change that.
>> 
>> KVM has this weird use case that you can map anything from /dev/mem
>> into the guest. pfn_valid() is not a reliable check whether the memmap
>> was initialized and can be touched. pfn_to_online_page() makes sure
>> that we have an initialized memmap (and don't have ZONE_DEVICE memory).
>> 
>> Rewrite is_invalid_reserved_pfn() similar to kvm_is_reserved_pfn() to make
>> sure the function produces the same result once we stop setting ZONE_DEVICE
>> pages PG_reserved.
>> 
>> Cc: Alex Williamson 
>> Cc: Cornelia Huck 
>> Signed-off-by: David Hildenbrand 
>> ---
>> drivers/vfio/vfio_iommu_type1.c | 10 --
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/vfio/vfio_iommu_type1.c 
>> b/drivers/vfio/vfio_iommu_type1.c
>> index 2ada8e6cdb88..f8ce8c408ba8 100644
>> --- a/drivers/vfio/vfio_iommu_type1.c
>> +++ b/drivers/vfio/vfio_iommu_type1.c
>> @@ -299,9 +299,15 @@ static int vfio_lock_acct(struct vfio_dma *dma, long 
>> npage, bool async)
>>  */
>> static bool is_invalid_reserved_pfn(unsigned long pfn)
>> {
>> -   if (pfn_valid(pfn))
>> -   return PageReserved(pfn_to_page(pfn));
>> +   struct page *page = pfn_to_online_page(pfn);
> 
> Ugh, I just realized this is not a safe conversion until
> pfn_to_online_page() is moved over to subsection granularity. As it
> stands it will return true for any ZONE_DEVICE pages that share a
> section with boot memory.

That should not happen right now and I commented back when you introduced 
subsection support that I don’t want to have ZONE_DEVICE mixed with online 
pages in a section. Having memory block devices that partially span ZONE_DEVICE 
would be ... really weird. With something like pfn_active() - as discussed - we 
could at least make this check work - but I am not sure if we really want to go 
down that path. In the worst case, some MB of RAM are lost ... I guess this 
needs more thought.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v1 04/10] vfio/type1: Prepare is_invalid_reserved_pfn() for PG_reserved changes

2019-11-07 Thread Dan Williams
On Thu, Nov 7, 2019 at 2:07 PM David Hildenbrand  wrote:
>
> On 07.11.19 19:22, David Hildenbrand wrote:
> >
> >
> >> Am 07.11.2019 um 16:40 schrieb Dan Williams :
> >>
> >> On Thu, Oct 24, 2019 at 5:12 AM David Hildenbrand  
> >> wrote:
> >>>
> >>> Right now, ZONE_DEVICE memory is always set PG_reserved. We want to
> >>> change that.
> >>>
> >>> KVM has this weird use case that you can map anything from /dev/mem
> >>> into the guest. pfn_valid() is not a reliable check whether the memmap
> >>> was initialized and can be touched. pfn_to_online_page() makes sure
> >>> that we have an initialized memmap (and don't have ZONE_DEVICE memory).
> >>>
> >>> Rewrite is_invalid_reserved_pfn() similar to kvm_is_reserved_pfn() to make
> >>> sure the function produces the same result once we stop setting 
> >>> ZONE_DEVICE
> >>> pages PG_reserved.
> >>>
> >>> Cc: Alex Williamson 
> >>> Cc: Cornelia Huck 
> >>> Signed-off-by: David Hildenbrand 
> >>> ---
> >>> drivers/vfio/vfio_iommu_type1.c | 10 --
> >>> 1 file changed, 8 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/vfio/vfio_iommu_type1.c 
> >>> b/drivers/vfio/vfio_iommu_type1.c
> >>> index 2ada8e6cdb88..f8ce8c408ba8 100644
> >>> --- a/drivers/vfio/vfio_iommu_type1.c
> >>> +++ b/drivers/vfio/vfio_iommu_type1.c
> >>> @@ -299,9 +299,15 @@ static int vfio_lock_acct(struct vfio_dma *dma, long 
> >>> npage, bool async)
> >>>   */
> >>> static bool is_invalid_reserved_pfn(unsigned long pfn)
> >>> {
> >>> -   if (pfn_valid(pfn))
> >>> -   return PageReserved(pfn_to_page(pfn));
> >>> +   struct page *page = pfn_to_online_page(pfn);
> >>
> >> Ugh, I just realized this is not a safe conversion until
> >> pfn_to_online_page() is moved over to subsection granularity. As it
> >> stands it will return true for any ZONE_DEVICE pages that share a
> >> section with boot memory.
> >
> > That should not happen right now and I commented back when you introduced 
> > subsection support that I don’t want to have ZONE_DEVICE mixed with online 
> > pages in a section. Having memory block devices that partially span 
> > ZONE_DEVICE would be ... really weird. With something like pfn_active() - 
> > as discussed - we could at least make this check work - but I am not sure 
> > if we really want to go down that path. In the worst case, some MB of RAM 
> > are lost ... I guess this needs more thought.
> >
>
> I just realized the "boot memory" part. Is that a real thing? IOW, can
> we have ZONE_DEVICE falling into a memory block (with holes)? I somewhat
> have doubts that this would work ...

One of the real world failure cases that started the subsection effect
is that Persistent Memory collides with System RAM on a 64MB boundary
on shipping platforms. System RAM ends on a 64MB boundary and due to a
lack of memory controller resources PMEM is mapped contiguously at the
end of that boundary. Some more details in the subsection cover letter
/ changelogs [1] [2]. It's not sufficient to just lose some memory,
that's the broken implementation that lead to the subsection work
because the lost memory may change from one boot to the next and
software can't reliably inject a padding that conforms to the x86
128MB section constraint.

Suffice to say I think we need your pfn_active() to get subsection
granularity pfn_to_online_page() before PageReserved() can be removed.

[1]: 
https://lore.kernel.org/linux-mm/156092349300.979959.17603710711957735135.st...@dwillia2-desk3.amr.corp.intel.com/
[2]: 
https://lore.kernel.org/linux-mm/156092354368.979959.6232443923440952359.st...@dwillia2-desk3.amr.corp.intel.com/

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

[Xen-devel] [xen-4.12-testing test] 143851: regressions - FAIL

2019-11-07 Thread osstest service owner
flight 143851 xen-4.12-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/143851/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-raw  19 guest-start/debian.repeat fail REGR. vs. 143190
 test-amd64-amd64-libvirt-vhd 17 guest-start/debian.repeat fail REGR. vs. 143190
 test-armhf-armhf-libvirt-raw 15 guest-start/debian.repeat fail REGR. vs. 143190
 test-armhf-armhf-xl-vhd 15 guest-start/debian.repeat fail REGR. vs. 143190

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

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

2019-11-07 Thread osstest service owner
flight 143848 linux-linus real [real]
http://logs.test-lab.xenproject.org/osstest/logs/143848/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-boot fail REGR. vs. 
133580
 test-amd64-i386-examine   8 reboot   fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 
133580
 test-amd64-i386-xl-raw7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-qemut-rhel6hvm-intel  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-qemuu-rhel6hvm-intel  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-ovmf-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 
133580
 test-amd64-i386-xl-qemut-win7-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-freebsd10-i386  7 xen-boot   fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-win10-i386  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-libvirt   7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-qemuu-rhel6hvm-amd  7 xen-boot   fail REGR. vs. 133580
 test-amd64-i386-qemut-rhel6hvm-amd  7 xen-boot   fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-debianhvm-amd64  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 133580
 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail REGR. vs. 133580
 test-amd64-i386-pair 10 xen-boot/src_hostfail REGR. vs. 133580
 test-amd64-i386-pair 11 xen-boot/dst_hostfail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail REGR. vs. 
133580
 test-amd64-i386-xl7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-freebsd10-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl-xsm7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-debianhvm-amd64  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-pvshim 7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-win10-i386  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-libvirt-xsm   7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-ws16-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-win7-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl-shadow 7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-ws16-amd64  7 xen-boot  fail REGR. vs. 133580
 test-arm64-arm64-examine11 examine-serial/bootloader fail REGR. vs. 133580
 test-amd64-amd64-xl-qcow2   19 guest-start/debian.repeat fail REGR. vs. 133580
 test-amd64-amd64-libvirt-vhd 17 guest-start/debian.repeat fail REGR. vs. 133580
 test-armhf-armhf-libvirt-raw 15 guest-start/debian.repeat fail REGR. vs. 133580
 test-armhf-armhf-xl-vhd 15 guest-start/debian.repeat fail REGR. vs. 133580

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10   fail REGR. vs. 133580

Tests which did not succeed, but are not blocking:
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  7 xen-boot fail baseline untested
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm  7 xen-boot fail baseline untested
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 133580
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 133580
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 133580
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 133580
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 133580
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 133580
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  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
 

Re: [Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts

2019-11-07 Thread Tian, Kevin
> From: Roger Pau Monné [mailto:roger@citrix.com]
> Sent: Monday, November 4, 2019 5:47 PM
> 
> On Sat, Nov 02, 2019 at 07:48:06AM +, Tian, Kevin wrote:
> > > From: Roger Pau Monné [mailto:roger@citrix.com]
> > > Sent: Thursday, October 31, 2019 11:23 PM
> > >
> > > On Thu, Oct 31, 2019 at 07:52:23AM -0700, Joe Jin wrote:
> > > > On 10/31/19 1:01 AM, Jan Beulich wrote:
> > > > > On 30.10.2019 19:01, Joe Jin wrote:
> > > > >> On 10/30/19 10:23 AM, Roger Pau Monné wrote:
> > > > >>> On Wed, Oct 30, 2019 at 09:38:16AM -0700, Joe Jin wrote:
> > > >  On 10/30/19 1:24 AM, Roger Pau Monné wrote:
> > > > > Can you try to add the following debug patch on top of the
> existing
> > > > > one and report the output that you get on the Xen console?
> > > > 
> > > >  Applied debug patch and run the test again, not of any log
> printed,
> > > >  attached Xen log on serial console, seems pi_update_irte() not
> been
> > > >  called for iommu_intpost was false.
> > > > >>>
> > > > >>> I have to admit I'm lost at this point. Does it mean the original
> > > > >>> issue had nothing to do with posted interrupts?
> > > > >>
> > > > >> Looks when inject irq by vlapic_set_irq(), it checked by
> > > > >> hvm_funcs.deliver_posted_intr rather than iommu_intpost:
> > > > >>
> > > > >>  176 if ( hvm_funcs.deliver_posted_intr )
> > > > >>  177 hvm_funcs.deliver_posted_intr(target, vec);
> > > > >>
> > > > >> And deliver_posted_intr() would be there, when vmx enabled:
> > > > >>
> > > > >> (XEN) HVM: VMX enabled
> > > > >> (XEN) HVM: Hardware Assisted Paging (HAP) detected
> > > > >> (XEN) HVM: HAP page sizes: 4kB, 2MB, 1GB
> > > > >
> > > > > I can't see the connection. start_vmx() has
> > > > >
> > > > > if ( cpu_has_vmx_posted_intr_processing )
> > > > > {
> > > > > alloc_direct_apic_vector(_intr_vector,
> > > pi_notification_interrupt);
> > > > > if ( iommu_intpost )
> > > > > alloc_direct_apic_vector(_wakeup_vector,
> > > pi_wakeup_interrupt);
> > > > >
> > > > > vmx_function_table.deliver_posted_intr =
> vmx_deliver_posted_intr;
> > > > > vmx_function_table.sync_pir_to_irr = vmx_sync_pir_to_irr;
> > > > > vmx_function_table.test_pir= vmx_test_pir;
> > > > > }
> > > > >
> > > > > i.e. the hook is present only when posted interrupts are
> > > > > available in general. I.e. also with just CPU-side posted
> > > > > interrupts, yes, which gets confirmed by your "apicv=0"
> > > > > test. Yet with just CPU-side posted interrupts I'm
> > > > > struggling again to understand your original problem
> > > > > description, and the need to fiddle with IOMMU side code.
> > > >
> > > > Yes, on my test env, cpu_has_vmx_posted_intr_processing == true &&
> > > iommu_intpost == false,
> > > > with this, posted interrupts been enabled.
> > >
> > > I'm still quite lost. My reading of the Intel VT-d spec is that the
> > > posted interrupt descriptor (which contains the PIRR) is used in
> > > conjunction with a posted interrupt remapping entry in the iommu, so
> > > that interrupts get recorded in the PIRR and later synced by the
> > > hypervisor into the vlapic IRR when resuming the virtual CPU.
> >
> > there are two parts. Intel first implements CPU posted interrupt,
> > which allows one CPU to post IPI into non-root context in another
> > CPU through posted interrupt descriptor. Later VT-d posted
> > interrupt comes, which use interrupt remapping entry and the
> > same posted interrupt descriptor (using more fields) to convert
> > a device interrupt into a posted interrupt. The posting process is
> > same on the dest CPU, regardless of whether it's from another CPU
> > or a device.
> 
> Thanks for the description.
> 
> So the problem reported by Jin happens when using CPU posted
> interrupts but not VT-d posted interrupts, in which case there
> shouldn't be a need to sync PIRR with IRR when interrupts from a
> passthrough device are reconfigured, because interrupts from that
> device shouldn't end up signaled in PIRR because VT-d posted
> interrupts is not being used.
> 
> Do interrupts from passthrough devices end up signaled in the posted
> interrupt descriptor PIRR field when not using VT-d posted
> interrupts but using CPU posted interrupts?

No. If VT-d posted interrupt is disabled, interrupts from passthrough
devices don't go through posted interrupt descriptor. But after hypervisor
serves the interrupt and when it decides to inject a virtual interrupt into
the guest, PIRR will be updated if CPU posted interrupt is enabled.

> 
> From my reading of your description above when using CPU posted
> interrupts only the vectors signaled in the PIRR field should belong
> to IPIs from other vCPUs?
> 

I didn't understand your question.

Thanks
Kevin

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

Re: [Xen-devel] [PATCH v2 02/15] mm/mmu_notifier: add an interval tree notifier

2019-11-07 Thread Jerome Glisse
On Fri, Nov 08, 2019 at 12:32:25AM +, Jason Gunthorpe wrote:
> On Thu, Nov 07, 2019 at 04:04:08PM -0500, Jerome Glisse wrote:
> > On Thu, Nov 07, 2019 at 08:11:06PM +, Jason Gunthorpe wrote:
> > > On Wed, Nov 06, 2019 at 09:08:07PM -0500, Jerome Glisse wrote:
> > > 
> > > > > 
> > > > > Extra credit: IMHO, this clearly deserves to all be in a new 
> > > > > mmu_range_notifier.h
> > > > > header file, but I know that's extra work. Maybe later as a follow-up 
> > > > > patch,
> > > > > if anyone has the time.
> > > > 
> > > > The range notifier should get the event too, it would be a waste, i 
> > > > think it is
> > > > an oversight here. The release event is fine so NAK to you separate 
> > > > event. Event
> > > > is really an helper for notifier i had a set of patch for nouveau to 
> > > > leverage
> > > > this i need to resucite them. So no need to split thing, i would just 
> > > > forward
> > > > the event ie add event to mmu_range_notifier_ops.invalidate() i failed 
> > > > to catch
> > > > that in v1 sorry.
> > > 
> > > I think what you mean is already done?
> > > 
> > > struct mmu_range_notifier_ops {
> > >   bool (*invalidate)(struct mmu_range_notifier *mrn,
> > >  const struct mmu_notifier_range *range,
> > >  unsigned long cur_seq);
> > 
> > Yes it is sorry, i got confuse with mmu_range_notifier and 
> > mmu_notifier_range :)
> > It is almost a palyndrome structure ;)
> 
> Lets change the name then, this is clearly not working. I'll reflow
> everything tomorrow

Semantic patch to do that run from your linux kernel directory with your patch
applied (you can run it one patch after the other and the git commit -a --fixup 
HEAD)

spatch --sp-file name-of-the-file-below --dir . --all-includes --in-place

%< --
@@
@@
struct
-mmu_range_notifier
+mmu_interval_notifier

@@
@@
struct
-mmu_range_notifier
+mmu_interval_notifier
{...};

// Change mrn name to mmu_in
@@
struct mmu_interval_notifier *mrn;
@@
-mrn
+mmu_in

@@
identifier fn;
@@
fn(..., 
-struct mmu_interval_notifier *mrn,
+struct mmu_interval_notifier *mmu_in,
...) {...}
-- >%

You need coccinelle (which provides spatch). It is untested but it should work
also i could not come up with a nice name to update mrn as min is way too
confusing. If you have better name feel free to use it.

Oh and coccinelle is pretty clever about code formating so it should do a good
jobs at keeping things nicely formated and align.

Cheers,
Jérôme


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

Re: [Xen-devel] [PATCH v2 02/15] mm/mmu_notifier: add an interval tree notifier

2019-11-07 Thread Jason Gunthorpe
On Thu, Nov 07, 2019 at 04:04:08PM -0500, Jerome Glisse wrote:
> On Thu, Nov 07, 2019 at 08:11:06PM +, Jason Gunthorpe wrote:
> > On Wed, Nov 06, 2019 at 09:08:07PM -0500, Jerome Glisse wrote:
> > 
> > > > 
> > > > Extra credit: IMHO, this clearly deserves to all be in a new 
> > > > mmu_range_notifier.h
> > > > header file, but I know that's extra work. Maybe later as a follow-up 
> > > > patch,
> > > > if anyone has the time.
> > > 
> > > The range notifier should get the event too, it would be a waste, i think 
> > > it is
> > > an oversight here. The release event is fine so NAK to you separate 
> > > event. Event
> > > is really an helper for notifier i had a set of patch for nouveau to 
> > > leverage
> > > this i need to resucite them. So no need to split thing, i would just 
> > > forward
> > > the event ie add event to mmu_range_notifier_ops.invalidate() i failed to 
> > > catch
> > > that in v1 sorry.
> > 
> > I think what you mean is already done?
> > 
> > struct mmu_range_notifier_ops {
> > bool (*invalidate)(struct mmu_range_notifier *mrn,
> >const struct mmu_notifier_range *range,
> >unsigned long cur_seq);
> 
> Yes it is sorry, i got confuse with mmu_range_notifier and mmu_notifier_range 
> :)
> It is almost a palyndrome structure ;)

Lets change the name then, this is clearly not working. I'll reflow
everything tomorrow

Jason

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

Re: [Xen-devel] Community Call Minutes: Nov 7

2019-11-07 Thread Lars Kurth
Hi all,
minutes are attached or at 
https://cryptpad.fr/pad/#/2/pad/view/7l3a4mhZTU4xs0GE415OXiAj0ScKl39xdQ9wm0cwASs/embed/present/
Regards
Lars

On 07/11/2019, 07:24, "Lars Kurth"  wrote:

Hi all,

quick reminder re agenda items. What I have so far is at 
https://cryptpad.fr/pad/#/2/pad/edit/SkeU+Z5J9WIIU9ZsXlojiXcQ/ 

C.1) Any more 4.13 coordination (Juergen)
C.2) Volunteers/suggestions for Release Managers for 4.13 (Lars / Juergen)
C.3) 4.13 Release Notes / Blog Post / Feature List - needs review (Lars)
See 
https://docs.google.com/document/d/1EpigvxDzeoc1dOMFwQ9itDXY4vY7nvxPiLGcNQQmX28/edit?usp=sharing

AOB
1) Travel and discussions
Rich Persaud, Christopher Clark & Daniel Smith will be in Cambridge Dec 10 
pm & 11 am 
Discussions are planned around a number of topics such as state of XSM, 
DomB proposal as a secure means to start an L0/L1 configuration, KCONFIG for L0 
version of Xen, etc.
Citrix will host, but others are welcome to join (please contact Lars for 
logistics)

Feel free to request additional items

Regards
Lars

On 04/11/2019, 05:37, "Lars Kurth"  wrote:

Dear community members,
 
please send me agenda items for next week’s community call. A draft 
agenda is at https://cryptpad.fr/pad/#/2/pad/edit/SkeU+Z5J9WIIU9ZsXlojiXcQ/
Please add agenda items to the document or reply to this e-mail
Note that I am on PTO today and tomorrow
 
Last month’s minutes are at 
https://cryptpad.fr/pad/#/2/pad/edit/4FGEw81flPUiivkjkuvQJ-CK/
 
Best Regards
Lars

## Meeting time (please double check the times
15:00 - 16:00 UTC
07:00 - 08:00 PST (San Francisco) - sorry for the early time slot. If 
this is a problem, let's discuss at the call
10:00 - 11:00 EST (New York)
15:00 - 16:00 FMT (London)
16:00 - 17:00 CET (Berlin)
23:00 - 22:00 CST (Beijing)
Further International meeting times: 
https://www.timeanddate.com/worldclock/meetingdetails.html?year=2018=11=7=15=0=0=224=24=179=136=37=33

## Dial in details
Web: https://www.gotomeet.me/larskurth

You can also dial in using your phone.
Access Code: 906-886-965

China (Toll Free): 4008 811084
Germany: +49 692 5736 7317
Poland (Toll Free): 00 800 1124759
United Kingdom: +44 330 221 0088
United States: +1 (571) 317-3129

More phone numbers
Australia: +61 2 9087 3604
Austria: +43 7 2081 5427
Argentina (Toll Free): 0 800 444 3375
Bahrain (Toll Free): 800 81 111
Belarus (Toll Free): 8 820 0011 0400
Belgium: +32 28 93 7018
Brazil (Toll Free): 0 800 047 4906
Bulgaria (Toll Free): 00800 120 4417
Canada: +1 (647) 497-9391
Chile (Toll Free): 800 395 150
Colombia (Toll Free): 01 800 518 4483
Czech Republic (Toll Free): 800 500448
Denmark: +45 32 72 03 82
Finland: +358 923 17 0568
France: +33 170 950 594
Greece (Toll Free): 00 800 4414 3838
Hong Kong (Toll Free): 30713169
Hungary (Toll Free): (06) 80 986 255
Iceland (Toll Free): 800 7204
India (Toll Free): 18002669272
Indonesia (Toll Free): 007 803 020 5375
Ireland: +353 15 360 728
Israel (Toll Free): 1 809 454 830
Italy: +39 0 247 92 13 01
Japan (Toll Free): 0 120 663 800
Korea, Republic of (Toll Free): 00798 14 207 4914
Luxembourg (Toll Free): 800 85158
Malaysia (Toll Free): 1 800 81 6854
Mexico (Toll Free): 01 800 522 1133
Netherlands: +31 207 941 377
New Zealand: +64 9 280 6302
Norway: +47 21 93 37 51
Panama (Toll Free): 00 800 226 7928
Peru (Toll Free): 0 800 77023
Philippines (Toll Free): 1 800 1110 1661
Portugal (Toll Free): 800 819 575
Romania (Toll Free): 0 800 410 029
Russian Federation (Toll Free): 8 800 100 6203
Saudi Arabia (Toll Free): 800 844 3633
Singapore (Toll Free): 18007231323
South Africa (Toll Free): 0 800 555 447
Spain: +34 932 75 2004
Sweden: +46 853 527 827
Switzerland: +41 225 4599 78
Taiwan (Toll Free): 0 800 666 854
Thailand (Toll Free): 001 800 011 023
Turkey (Toll Free): 00 800 4488 23683
Ukraine (Toll Free): 0 800 50 1733
United Arab Emirates (Toll Free): 800 044 40439
Uruguay (Toll Free): 0004 019 1018
Viet Nam (Toll Free): 122 80 481

First GoToMeeting? Let's do a quick system check:
https://link.gotomeeting.com/system-check








2019-11 Community Call.pdf
Description: 2019-11 Community Call.pdf
___
Xen-devel mailing list

Re: [Xen-devel] [PATCH v2 09/15] xen/gntdev: use mmu_range_notifier_insert

2019-11-07 Thread Boris Ostrovsky
On 11/7/19 3:36 PM, Jason Gunthorpe wrote:
> On Tue, Nov 05, 2019 at 10:16:46AM -0500, Boris Ostrovsky wrote:
>
>>> So, I suppose it can be relaxed to a null test and a WARN_ON that it
>>> hasn't changed?
>> You mean
>>
>> if (use_ptemod) {
>>     WARN_ON(map->vma != vma);
>>     ...
>>
>>
>> Yes, that sounds good.
> I amended my copy of the patch with the above, has this rework shown
> signs of working?

Yes, it works fine.

But please don't forget notifier ops initialization.

With those two changes,

Reviewed-by: Boris Ostrovsky 

>
> @@ -436,7 +436,8 @@ static void gntdev_vma_close(struct vm_area_struct *vma)
> struct gntdev_priv *priv = file->private_data;
>  
> pr_debug("gntdev_vma_close %p\n", vma);
> -   if (use_ptemod && map->vma == vma) {
> +   if (use_ptemod) {
> +   WARN_ON(map->vma != vma);
> mmu_range_notifier_remove(>notifier);
> map->vma = NULL;
> }
>
> Jason


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

[Xen-devel] Call for new Release Manager for Xen 4.14+

2019-11-07 Thread Lars Kurth
Dear Community Members, 

Juergen will be stepping down as Release Manager after Xen 4.13 has been 
delivered, following the 4.11 and 4.12 release. Release managers prior to 
Juergen were Julien Grall, Konrad Wilk, Wei Liu and George Dunlap. We are 
looking for active community members to follow in previous release managers 
footsteps. I also wanted to thank Juergen for performing the role. 

We have discussed with a number of people, however Wei made the very valid 
point that we should make an announcement about the role on the list.  In terms 
of effort, the effort required prior to the release is relatively low (1-2 days 
a month), however in the last two months of the release goes up to 1-2 days per 
week. Typically release managers manage 2-3 releases.

What is involved in the role is described here: 
http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=docs/process/xen-release-management.pandoc;h=d6abc90a0248b769161bce79e8dc6904c654904a;hb=HEAD

If you are a community member that feels the release manager role would be a 
good match for you, please contact me: also feel free to ask me or previous 
release managers any questions

Best Regards
Lars

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

Re: [Xen-devel] [PATCH v1 04/10] vfio/type1: Prepare is_invalid_reserved_pfn() for PG_reserved changes

2019-11-07 Thread David Hildenbrand

On 07.11.19 19:22, David Hildenbrand wrote:




Am 07.11.2019 um 16:40 schrieb Dan Williams :

On Thu, Oct 24, 2019 at 5:12 AM David Hildenbrand  wrote:


Right now, ZONE_DEVICE memory is always set PG_reserved. We want to
change that.

KVM has this weird use case that you can map anything from /dev/mem
into the guest. pfn_valid() is not a reliable check whether the memmap
was initialized and can be touched. pfn_to_online_page() makes sure
that we have an initialized memmap (and don't have ZONE_DEVICE memory).

Rewrite is_invalid_reserved_pfn() similar to kvm_is_reserved_pfn() to make
sure the function produces the same result once we stop setting ZONE_DEVICE
pages PG_reserved.

Cc: Alex Williamson 
Cc: Cornelia Huck 
Signed-off-by: David Hildenbrand 
---
drivers/vfio/vfio_iommu_type1.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 2ada8e6cdb88..f8ce8c408ba8 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -299,9 +299,15 @@ static int vfio_lock_acct(struct vfio_dma *dma, long 
npage, bool async)
  */
static bool is_invalid_reserved_pfn(unsigned long pfn)
{
-   if (pfn_valid(pfn))
-   return PageReserved(pfn_to_page(pfn));
+   struct page *page = pfn_to_online_page(pfn);


Ugh, I just realized this is not a safe conversion until
pfn_to_online_page() is moved over to subsection granularity. As it
stands it will return true for any ZONE_DEVICE pages that share a
section with boot memory.


That should not happen right now and I commented back when you introduced 
subsection support that I don’t want to have ZONE_DEVICE mixed with online 
pages in a section. Having memory block devices that partially span ZONE_DEVICE 
would be ... really weird. With something like pfn_active() - as discussed - we 
could at least make this check work - but I am not sure if we really want to go 
down that path. In the worst case, some MB of RAM are lost ... I guess this 
needs more thought.



I just realized the "boot memory" part. Is that a real thing? IOW, can 
we have ZONE_DEVICE falling into a memory block (with holes)? I somewhat 
have doubts that this would work ...


--

Thanks,

David / dhildenb


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

[Xen-devel] [linux-4.4 test] 143846: regressions - FAIL

2019-11-07 Thread osstest service owner
flight 143846 linux-4.4 real [real]
http://logs.test-lab.xenproject.org/osstest/logs/143846/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qcow2   19 guest-start/debian.repeat fail REGR. vs. 139698
 test-amd64-amd64-libvirt-vhd 17 guest-start/debian.repeat fail REGR. vs. 139698
 test-amd64-i386-xl-raw  19 guest-start/debian.repeat fail REGR. vs. 139698
 test-armhf-armhf-libvirt-raw 15 guest-start/debian.repeat fail REGR. vs. 139698
 test-armhf-armhf-xl-vhd 15 guest-start/debian.repeat fail REGR. vs. 139698
 test-amd64-amd64-xl-pvshim 18 guest-localmigrate/x10 fail in 143425 REGR. vs. 
139698

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeat fail in 143548 pass in 
143846
 test-armhf-armhf-xl-rtds 12 guest-start  fail in 143646 pass in 143846
 test-amd64-amd64-xl-pvshim   12 guest-startfail pass in 143425
 test-armhf-armhf-xl-credit2   7 xen-boot   fail pass in 143548
 test-armhf-armhf-libvirt 19 leak-check/check   fail pass in 143646

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10   fail REGR. vs. 139698

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-credit2 13 migrate-support-check fail in 143425 never pass
 test-armhf-armhf-xl-credit2 14 saverestore-support-check fail in 143425 never 
pass
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail never pass
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-amd64-xl-pvhv2-amd 12 guest-start  fail  never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   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-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never 

Re: [Xen-devel] [PATCH v2 02/15] mm/mmu_notifier: add an interval tree notifier

2019-11-07 Thread Jerome Glisse
On Thu, Nov 07, 2019 at 08:11:06PM +, Jason Gunthorpe wrote:
> On Wed, Nov 06, 2019 at 09:08:07PM -0500, Jerome Glisse wrote:
> 
> > > 
> > > Extra credit: IMHO, this clearly deserves to all be in a new 
> > > mmu_range_notifier.h
> > > header file, but I know that's extra work. Maybe later as a follow-up 
> > > patch,
> > > if anyone has the time.
> > 
> > The range notifier should get the event too, it would be a waste, i think 
> > it is
> > an oversight here. The release event is fine so NAK to you separate event. 
> > Event
> > is really an helper for notifier i had a set of patch for nouveau to 
> > leverage
> > this i need to resucite them. So no need to split thing, i would just 
> > forward
> > the event ie add event to mmu_range_notifier_ops.invalidate() i failed to 
> > catch
> > that in v1 sorry.
> 
> I think what you mean is already done?
> 
> struct mmu_range_notifier_ops {
>   bool (*invalidate)(struct mmu_range_notifier *mrn,
>  const struct mmu_notifier_range *range,
>  unsigned long cur_seq);

Yes it is sorry, i got confuse with mmu_range_notifier and mmu_notifier_range :)
It is almost a palyndrome structure ;)

> 
> > No it is always odd, you must call mmu_range_set_seq() only from the
> > op->invalidate_range() callback at which point the seq is odd. As well
> > when mrn is added and its seq first set it is set to an odd value
> > always. Maybe the comment, should read:
> > 
> >  * mrn->invalidate_seq is always, yes always, set to an odd value. This 
> > ensures
> > 
> > To stress that it is not an error.
> 
> I went with this:
> 
>   /*
>* mrn->invalidate_seq must always be set to an odd value via
>* mmu_range_set_seq() using the provided cur_seq from
>* mn_itree_inv_start_range(). This ensures that if seq does wrap we
>* will always clear the below sleep in some reasonable time as
>* mmn_mm->invalidate_seq is even in the idle state.
>*/

Yes fine with me.

[...]

> > > > +   might_lock(>mmap_sem);
> > > > +
> > > > +   mmn_mm = smp_load_acquire(>mmu_notifier_mm);
> > > 
> > > What does the above pair with? Should have a comment that specifies that.
> > 
> > It was discussed in v1 but maybe a comment of what was said back then would
> > be helpful. Something like:
> > 
> > /*
> >  * We need to insure that all writes to mm->mmu_notifier_mm are visible 
> > before
> >  * any checks we do on mmn_mm below as otherwise CPU might re-order write 
> > done
> >  * by another CPU core to mm->mmu_notifier_mm structure fields after the 
> > read
> >  * belows.
> >  */
> 
> This comment made it, just at the store side:
> 
>   /*
>* Serialize the update against mmu_notifier_unregister. A
>* side note: mmu_notifier_release can't run concurrently with
>* us because we hold the mm_users pin (either implicitly as
>* current->mm or explicitly with get_task_mm() or similar).
>* We can't race against any other mmu notifier method either
>* thanks to mm_take_all_locks().
>*
>* release semantics on the initialization of the mmu_notifier_mm's
>  * contents are provided for unlocked readers.  acquire can only be
>  * used while holding the mmgrab or mmget, and is safe because once
>  * created the mmu_notififer_mm is not freed until the mm is
>  * destroyed.  As above, users holding the mmap_sem or one of the
>  * mm_take_all_locks() do not need to use acquire semantics.
>*/
>   if (mmu_notifier_mm)
>   smp_store_release(>mmu_notifier_mm, mmu_notifier_mm);
> 
> Which I think is really overly belaboring the typical smp
> store/release pattern, but people do seem unfamiliar with them...

Perfect with me. I think also sometimes you forgot what memory model is
and thus store/release pattern do, i know i do and i need to refresh my
mind.

Cheers,
Jérôme


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

Re: [Xen-devel] [PATCH v2 02/15] mm/mmu_notifier: add an interval tree notifier

2019-11-07 Thread John Hubbard

On 11/7/19 12:06 PM, Jason Gunthorpe wrote:
...


Also, it is best moved down to be next to the new MNR structs, so that all the
MNR stuff is in one group.


I agree with Jerome, this enum is part of the 'struct
mmu_notifier_range' (ie the description of the invalidation) and it
doesn't really matter that only these new notifiers can be called with
this type, it is still part of the mmu_notifier_range.



OK.


The comment already says it only applies to the mmu_range_notifier
scheme..


  #define MMU_NOTIFIER_RANGE_BLOCKABLE (1 << 0)
@@ -222,6 +228,26 @@ struct mmu_notifier {
unsigned int users;
  };
  


That should also be moved down, next to the new structs.


Which this?


I was referring to MMU_NOTIFIER_RANGE_BLOCKABLE, above. Trying
to put all the new range notifier stuff in one place. But maybe not,
if these are really not as separate as I thought.




+/**
+ * struct mmu_range_notifier_ops
+ * @invalidate: Upon return the caller must stop using any SPTEs within this
+ *  range, this function can sleep. Return false if blocking was
+ *  required but range is non-blocking
+ */


How about this (I'm not sure I fully understand the return value, though):

/**
  * struct mmu_range_notifier_ops
  * @invalidate: Upon return the caller must stop using any SPTEs within this
  * range.
  *
  * This function is permitted to sleep.
  *
  * @Return: false if blocking was required, but @range is
  * non-blocking.
  *
  */


Is this kdoc format for function pointers?


heh, I'm sort of winging it, I'm not sure how function pointers are supposed
to be documented in kdoc. Actually the only key take-away here is to write

"This function can sleep"

as a separate sentence..

...

c) Rename new one. Ideas:

 struct mmu_interval_notifier
 struct mmu_range_intersection
 ...other ideas?


This odd duality has already cause some confusion, but names here are
hard.  mmu_interval_notifier is the best alternative I've heard.

Changing this name is a lot of work - are we happy
'mmu_interval_notifier' is the right choice?



Yes, it's my favorite too. I'd vote for going with that.

...



OK, this either needs more documentation and assertions, or a different
approach. Because I see addition, subtraction, AND, OR and booleans
all being applied to this field, and it's darn near hopeless to figure
out whether or not it really is even or odd at the right times.


This is a standard design for a seqlock scheme and follows the
existing design of the linux seq lock.

The lower bit indicates the lock'd state and the upper bits indicate
the generation of the lock

The operations on the lock itself are then:
seq |= 1  # Take the lock
seq++ # Release an acquired lock
seq & 1   # True if locked

Which is how this is written


Very nice, would you be open to putting that into (any) one of the comment
headers? That's an unusually clear and concise description:

/*
 * This is a standard design for a seqlock scheme and follows the
 * existing design of the linux seq lock.
 *
 * The lower bit indicates the lock'd state and the upper bits indicate
 * the generation of the lock
 *
 * The operations on the lock itself are then:
 *seq |= 1  # Take the lock
 *seq++ # Release an acquired lock
 *seq & 1   # True if locked
 */





Different approach: why not just add a mmn_mm->is_invalidating
member variable? It's not like you're short of space in that struct.


Splitting it makes alot of stuff more complex and unnatural.



OK, agreed.


The ops above could be put in inline wrappers, but they only occur
only in functions already called mn_itree_inv_start_range() and
mn_itree_inv_end() and mn_itree_is_invalidating().

There is the one 'take the lock' outlier in
__mmu_range_notifier_insert() though


+static void mn_itree_inv_end(struct mmu_notifier_mm *mmn_mm)
+{
+   struct mmu_range_notifier *mrn;
+   struct hlist_node *next;
+   bool need_wake = false;
+
+   spin_lock(_mm->lock);
+   if (--mmn_mm->active_invalidate_ranges ||
+   !mn_itree_is_invalidating(mmn_mm)) {
+   spin_unlock(_mm->lock);
+   return;
+   }
+
+   mmn_mm->invalidate_seq++;


Is this the right place for an assertion that this is now an even value?


Yes, but I'm reluctant to add such a runtime check on this fastish path..
How about a comment?


Sure.




+   need_wake = true;
+
+   /*
+* The inv_end incorporates a deferred mechanism like
+* rtnl_lock(). Adds and removes are queued until the final inv_end


Let me point out that rtnl_lock() itself is a one-liner that calls mutex_lock().
But I suppose if one studies that file closely there is more. :)


Lets change that to rtnl_unlock() then



Thanks :)


...

+* mrn->invalidate_seq is always set to an odd value. This ensures


This claim just looks wrong the first N times one reads the code, given that

Re: [Xen-devel] [PATCH v2 09/15] xen/gntdev: use mmu_range_notifier_insert

2019-11-07 Thread Jason Gunthorpe
On Tue, Nov 05, 2019 at 10:16:46AM -0500, Boris Ostrovsky wrote:

> > So, I suppose it can be relaxed to a null test and a WARN_ON that it
> > hasn't changed?
> 
> You mean
> 
> if (use_ptemod) {
>     WARN_ON(map->vma != vma);
>     ...
> 
> 
> Yes, that sounds good.

I amended my copy of the patch with the above, has this rework shown
signs of working?

@@ -436,7 +436,8 @@ static void gntdev_vma_close(struct vm_area_struct *vma)
struct gntdev_priv *priv = file->private_data;
 
pr_debug("gntdev_vma_close %p\n", vma);
-   if (use_ptemod && map->vma == vma) {
+   if (use_ptemod) {
+   WARN_ON(map->vma != vma);
mmu_range_notifier_remove(>notifier);
map->vma = NULL;
}

Jason

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

Re: [Xen-devel] [PATCH v2 02/15] mm/mmu_notifier: add an interval tree notifier

2019-11-07 Thread Jason Gunthorpe
On Wed, Nov 06, 2019 at 09:08:07PM -0500, Jerome Glisse wrote:

> > 
> > Extra credit: IMHO, this clearly deserves to all be in a new 
> > mmu_range_notifier.h
> > header file, but I know that's extra work. Maybe later as a follow-up patch,
> > if anyone has the time.
> 
> The range notifier should get the event too, it would be a waste, i think it 
> is
> an oversight here. The release event is fine so NAK to you separate event. 
> Event
> is really an helper for notifier i had a set of patch for nouveau to leverage
> this i need to resucite them. So no need to split thing, i would just forward
> the event ie add event to mmu_range_notifier_ops.invalidate() i failed to 
> catch
> that in v1 sorry.

I think what you mean is already done?

struct mmu_range_notifier_ops {
bool (*invalidate)(struct mmu_range_notifier *mrn,
   const struct mmu_notifier_range *range,
   unsigned long cur_seq);

> No it is always odd, you must call mmu_range_set_seq() only from the
> op->invalidate_range() callback at which point the seq is odd. As well
> when mrn is added and its seq first set it is set to an odd value
> always. Maybe the comment, should read:
> 
>  * mrn->invalidate_seq is always, yes always, set to an odd value. This 
> ensures
> 
> To stress that it is not an error.

I went with this:

/*
 * mrn->invalidate_seq must always be set to an odd value via
 * mmu_range_set_seq() using the provided cur_seq from
 * mn_itree_inv_start_range(). This ensures that if seq does wrap we
 * will always clear the below sleep in some reasonable time as
 * mmn_mm->invalidate_seq is even in the idle state.
 */

> > > + spin_lock(_mm->lock);
> > > + if (mmn_mm->active_invalidate_ranges) {
> > > + if (mn_itree_is_invalidating(mmn_mm))
> > > + hlist_add_head(>deferred_item,
> > > +_mm->deferred_list);
> > > + else {
> > > + mmn_mm->invalidate_seq |= 1;
> > > + interval_tree_insert(>interval_tree,
> > > +  _mm->itree);
> > > + }
> > > + mrn->invalidate_seq = mmn_mm->invalidate_seq;
> > > + } else {
> > > + WARN_ON(mn_itree_is_invalidating(mmn_mm));
> > > + mrn->invalidate_seq = mmn_mm->invalidate_seq - 1;
> > 
> > Ohhh, checkmate. I lose. Why is *subtracting* the right thing to do
> > for seq numbers here?  I'm acutely unhappy trying to figure this out.
> > I suspect it's another unfortunate side effect of trying to use the
> > lower bit of the seq number (even/odd) for something else.
> 
> If there is no mmn_mm->active_invalidate_ranges then it means that
> mmn_mm->invalidate_seq is even and thus mmn_mm->invalidate_seq - 1
> is an odd number which means that mrn->invalidate_seq is initialized
> to odd value and if you follow the rule for calling mmu_range_set_seq()
> then it will _always_ be an odd number and this close the loop with
> the above comments :)

The key thing is that it is an odd value that will take a long time
before mmn_mm->invalidate seq reaches it

> > > + might_lock(>mmap_sem);
> > > +
> > > + mmn_mm = smp_load_acquire(>mmu_notifier_mm);
> > 
> > What does the above pair with? Should have a comment that specifies that.
> 
> It was discussed in v1 but maybe a comment of what was said back then would
> be helpful. Something like:
> 
> /*
>  * We need to insure that all writes to mm->mmu_notifier_mm are visible before
>  * any checks we do on mmn_mm below as otherwise CPU might re-order write done
>  * by another CPU core to mm->mmu_notifier_mm structure fields after the read
>  * belows.
>  */

This comment made it, just at the store side:

/*
 * Serialize the update against mmu_notifier_unregister. A
 * side note: mmu_notifier_release can't run concurrently with
 * us because we hold the mm_users pin (either implicitly as
 * current->mm or explicitly with get_task_mm() or similar).
 * We can't race against any other mmu notifier method either
 * thanks to mm_take_all_locks().
 *
 * release semantics on the initialization of the mmu_notifier_mm's
 * contents are provided for unlocked readers.  acquire can only be
 * used while holding the mmgrab or mmget, and is safe because once
 * created the mmu_notififer_mm is not freed until the mm is
 * destroyed.  As above, users holding the mmap_sem or one of the
 * mm_take_all_locks() do not need to use acquire semantics.
 */
if (mmu_notifier_mm)
smp_store_release(>mmu_notifier_mm, mmu_notifier_mm);

Which I think is really overly belaboring the typical smp
store/release pattern, but people do seem unfamiliar with them...

Thanks,
Jason

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

Re: [Xen-devel] [PATCH v2 02/15] mm/mmu_notifier: add an interval tree notifier

2019-11-07 Thread Jason Gunthorpe
On Wed, Nov 06, 2019 at 04:23:21PM -0800, John Hubbard wrote:
 
> Nice design, I love the seq foundation! So far, I'm not able to spot anything
> actually wrong with the implementation, sorry about that. 

Alas :( I feel there must be a bug in here still, but onwards!

One of the main sad points was it didn't make sense to use the
existing seqlock/seqcount primitives as they have both the wrong write
concurrancy model and extra barriers that are not needed when it is
always manipulated under a spinlock
 
> 1. There is a rather severe naming overlap (not technically a naming conflict,
> but still) with existing mmn work, which already has, for example:
> 
> struct mmu_notifier_range
> 
> ...and you're adding:
> 
> struct mmu_range_notifier
> 
> ...so I'll try to help sort that out.

Yes, I've been sad about this too.

> So this should read:
> 
> enum mmu_range_notifier_event {
>   MMU_NOTIFY_RELEASE,
> };
> 
> ...assuming that we stay with "mmu_range_notifier" as a core name for this 
> whole thing.
> 
> Also, it is best moved down to be next to the new MNR structs, so that all the
> MNR stuff is in one group.

I agree with Jerome, this enum is part of the 'struct
mmu_notifier_range' (ie the description of the invalidation) and it
doesn't really matter that only these new notifiers can be called with
this type, it is still part of the mmu_notifier_range.

The comment already says it only applies to the mmu_range_notifier
scheme..

> >  #define MMU_NOTIFIER_RANGE_BLOCKABLE (1 << 0)
> > @@ -222,6 +228,26 @@ struct mmu_notifier {
> > unsigned int users;
> >  };
> >  
> 
> That should also be moved down, next to the new structs.

Which this?

> > +/**
> > + * struct mmu_range_notifier_ops
> > + * @invalidate: Upon return the caller must stop using any SPTEs within 
> > this
> > + *  range, this function can sleep. Return false if blocking 
> > was
> > + *  required but range is non-blocking
> > + */
> 
> How about this (I'm not sure I fully understand the return value, though):
> 
> /**
>  * struct mmu_range_notifier_ops
>  * @invalidate: Upon return the caller must stop using any SPTEs within this
>  *range.
>  *
>  *This function is permitted to sleep.
>  *
>  *@Return: false if blocking was required, but @range is
>  *non-blocking.
>  *
>  */

Is this kdoc format for function pointers?
 
> 
> > +struct mmu_range_notifier_ops {
> > +   bool (*invalidate)(struct mmu_range_notifier *mrn,
> > +  const struct mmu_notifier_range *range,
> > +  unsigned long cur_seq);
> > +};
> > +
> > +struct mmu_range_notifier {
> > +   struct interval_tree_node interval_tree;
> > +   const struct mmu_range_notifier_ops *ops;
> > +   struct hlist_node deferred_item;
> > +   unsigned long invalidate_seq;
> > +   struct mm_struct *mm;
> > +};
> > +
> 
> Again, now we have the new struct mmu_range_notifier, and the old 
> struct mmu_notifier_range, and it's not good.
> 
> Ideas:
> 
> a) Live with it.
> 
> b) (Discarded, too many callers): rename old one. Nope.
> 
> c) Rename new one. Ideas:
> 
> struct mmu_interval_notifier
> struct mmu_range_intersection
> ...other ideas?

This odd duality has already cause some confusion, but names here are
hard.  mmu_interval_notifier is the best alternative I've heard.

Changing this name is a lot of work - are we happy
'mmu_interval_notifier' is the right choice?

> > +/**
> > + * mmu_range_set_seq - Save the invalidation sequence
> 
> How about:
> 
>  * mmu_range_set_seq - Set the .invalidate_seq to a new value.

It is not a 'new value', it is a value that is provided to the
invalidate callback

> 
> > + * @mrn - The mrn passed to invalidate
> > + * @cur_seq - The cur_seq passed to invalidate
> > + *
> > + * This must be called unconditionally from the invalidate callback of a
> > + * struct mmu_range_notifier_ops under the same lock that is used to call
> > + * mmu_range_read_retry(). It updates the sequence number for later use by
> > + * mmu_range_read_retry().
> > + *
> > + * If the user does not call mmu_range_read_begin() or 
> > mmu_range_read_retry()
> 
> nit: "caller" is better than "user", when referring to...well, callers. 
> "user" 
> most often refers to user space, whereas a call stack and function calling is 
> clearly what you're referring to here (and in other places, especially "user 
> lock").

Done

> > +/**
> > + * mmu_range_check_retry - Test if a collision has occurred
> > + * mrn: The range under lock
> > + * seq: The return of the matching mmu_range_read_begin()
> > + *
> > + * This can be used in the critical section between mmu_range_read_begin() 
> > and
> > + * mmu_range_read_retry().  A return of true indicates an invalidation has
> > + * collided with this lock and a future mmu_range_read_retry() will return
> > + * true.
> > + *
> > + * False is not reliable and only suggests a collision has not happened. It
> 
> 

Re: [Xen-devel] [RFC] Documentation formats, licenses and file system structure

2019-11-07 Thread Lars Kurth
Hi all,

I have received informal advice

On 21/10/2019, 06:54, "Artem Mygaiev"  wrote:

>  Before we ask Xen FuSA contributors to invest in documentation to
> be presented as legally-valid evidence for certification, we should
> ask a certified lawyer for their formal opinion on the validity of:
> 
>   (a) applying a source code license (BSD) to documentation
> 
> There are also BSD documentation license variants which may be worth
> looking at

There is no LEGAL issue with using a source code license for documentation
Typically, community issues arise when the license is has a patent clause
which would act as a possible barrier to contributing to the docs (which should 
be low)

>   (b) moving text bidirectionally between source code (BSD) and
> documentation (any license)
>   (c) moving text bidirectionally between source code (BSD) and
> documentation (CC0)
> 
> I will raise this at the next SIG meeting

Fundamentally, you can’t move copyrightable content from any CC-BY-4/CC0 to BSD 
and vice versa without going through the process of changing a license

On the community call we discussed Andy's sphinx-docs. Andy made a strong case 
to keep the docset as CC-BY-4
It rests on the assumption that user docs will always be different from what's 
in code and thus there is no need to move anything which is copyrightable 
between code and the docs
Should that turn out to be wrong, there is still always the possibility of a 
mixed CC-BY-4 / BSD-2-Clause docset in future
So we are not painting ourselves into a corner

Regarding safety related docs, we discussed
* CC-BY-4 => this is likely to be problematic as many docs are coupled closely 
with source
* Dual CC-BY-4 / BSD-2-Clause licensing does not solve this problem
* BSD-2-Clause docs would enable docs that 

Thus, the most sensible approach for safety related docs would be to use a 
BSD-2-Clause license uniformly in that case

Regards
Lars

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

Re: [Xen-devel] [OSSTEST PATCH] make-flight: Drop all win10 tests in all flights

2019-11-07 Thread Jürgen Groß

On 07.11.19 18:52, Ian Jackson wrote:

These are failing and have been for some time and it does not appear
that anyone has the capability to fix them.  Running them in these
circumstances seems wasteful.

Effect is to drop test-*-win10-* jobs (checked with
standalone-generate-dump-flight-runvars).

CC: Sander Eikelenboom 
CC: Jürgen Groß 
Signed-off-by: Ian Jackson 


Release-acked-by: Juergen Gross 


Juergen

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

Re: [Xen-devel] [PATCH v2] tools/hotpug: only attempt to call 'ip route' if there is valid command

2019-11-07 Thread Wei Liu
On Thu, Nov 07, 2019 at 04:58:14PM +, p...@xen.org wrote:
> From: Paul Durrant 
> 
> The vif-route script should only call 'ip route' when 'ipcmd' has been
> set, otherwise it will fail due to an incorrect command string.
> 
> This patch also adds routes for 'tap' (i.e. emulated) devices as well as
> 'vif' (i.e. PV) devices by making use of the route metric. Emulated
> devices are used by HVM guests until they are unplugged, at which point the
> PV device becomes active. Thus 'tap' devices should get a higher priority
> (i.e. lower numbered) metric than 'vif' devices.
> 
> There is also one small whitespace fix.
> 
> NOTE: Empirically offline/online commands relate to 'vif' devices, and
>   add/remove commands relate to 'tap' devices. However, this patch
>   treats them equally and uses ${type_if} to distinguish.
> 
> Signed-off-by: Paul Durrant 
> ---
> Cc: Ian Jackson 
> Cc: Wei Liu 

Looks like you need to update your address book. :-)

> ---
>  tools/hotplug/Linux/vif-route | 22 +++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
>  mode change 100644 => 100755 tools/hotplug/Linux/vif-route
> 
> diff --git a/tools/hotplug/Linux/vif-route b/tools/hotplug/Linux/vif-route
> old mode 100644
> new mode 100755
> index c149ffc..e71acae
> --- a/tools/hotplug/Linux/vif-route
> +++ b/tools/hotplug/Linux/vif-route
> @@ -22,12 +22,16 @@ dir=$(dirname "$0")
>  main_ip=$(dom0_ip)
>  
>  case "${command}" in
> +add)
> +;&
>  online)
>  ifconfig ${dev} ${main_ip} netmask 255.255.255.255 up

Hmm... I think we may need to replace ifconfig with ip because now
distros (at least Debian and Arch) don't install ifconfig by default.

This can be done with a separate patch though.

>  echo 1 >/proc/sys/net/ipv4/conf/${dev}/proxy_arp
>  ipcmd='add'
>  cmdprefix=''
>  ;;
> +remove)
> +;&
>  offline)
>  do_without_error ifdown ${dev}
>  ipcmd='del'
> @@ -35,11 +39,23 @@ case "${command}" in
>  ;;
>  esac
>  

The list of action here is exhaustive per the comment of this file,
which means ipcmd will always be set. The test for ipcmd below becomes
unnecessary.

> -if [ "${ip}" ] ; then
> +case "${type_if}" in
> +tap)
> + metric=1
> + ;;
> +vif)
> + metric=2
> + ;;
> +*)
> + fatal "Unrecognised interface type ${type_if}"
> + ;;
> +esac
> +
> +if [ "${ipcmd}" ] ; then
>  # If we've been given a list of IP addresses, then add routes from dom0 
> to
>  # the guest using those addresses.

I _think_ testing ${ip} here is still the correct action. The comment
suggests there could be no ip given. If that assumption is not correct,
please fix the comment as well.

Wei.

>  for addr in ${ip} ; do
> -  ${cmdprefix} ip route ${ipcmd} ${addr} dev ${dev} src ${main_ip}
> +  ${cmdprefix} ip route ${ipcmd} ${addr} dev ${dev} src ${main_ip} 
> metric ${metric}
>  done
>  fi
>  
> @@ -50,5 +66,5 @@ call_hooks vif post
>  log debug "Successful vif-route ${command} for ${dev}."
>  if [ "${command}" = "online" ]
>  then
> -  success
> +success
>  fi
> -- 
> 2.7.4
> 
> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel

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

Re: [Xen-devel] [PATCH] tools: pygrub actually cross-compiles just fine

2019-11-07 Thread Wei Liu
On Thu, Nov 07, 2019 at 10:22:23AM -0800, Stefano Stabellini wrote:
> On Thu, 7 Nov 2019, Wei Liu wrote:
> > On Wed, Nov 06, 2019 at 08:10:47AM -0800, Stefano Stabellini wrote:
> > > On Wed, 6 Nov 2019, Wei Liu wrote:
> > > > On Tue, Nov 05, 2019 at 03:51:13PM -0800, Stefano Stabellini wrote:
> > > > > Actually, pygrub cross-compiles without issues. The cross-compilation
> > > > > work-around goes back to 2005 and it probably referred to PowerPC.
> > > > > 
> > > > > Remove the work-around now.
> > > > > 
> > > > > Signed-off-by: Stefano Stabellini 
> > > > > CC: Christopher Clark 
> > > > 
> > > > Presumably you tried to cross-compile it for Arm? It would be good to
> > > > mention that in the commit message.
> > > > 
> > > > I think the content of this patch is fine:
> > > 
> > > It cross-compiles fine for aarch64 on x86_64 with Yocto.  Although we
> > > don't do any cross-compilations in OSSTest as far as I know, so applying
> > > the patch won't break OSSTest, given the state of the release, I think
> > > it would be best to wait for the next merge window.
> > 
> > That's of course fine by me.
> 
> I'll resend the patch with a better commit message. It would be good to
> get an ack if you are OK with it (of course there will be no committing
> it right now.)

Acked-by: Wei Liu 

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

Re: [Xen-devel] [PATCH] tools: pygrub actually cross-compiles just fine

2019-11-07 Thread Stefano Stabellini
On Thu, 7 Nov 2019, Wei Liu wrote:
> On Wed, Nov 06, 2019 at 08:10:47AM -0800, Stefano Stabellini wrote:
> > On Wed, 6 Nov 2019, Wei Liu wrote:
> > > On Tue, Nov 05, 2019 at 03:51:13PM -0800, Stefano Stabellini wrote:
> > > > Actually, pygrub cross-compiles without issues. The cross-compilation
> > > > work-around goes back to 2005 and it probably referred to PowerPC.
> > > > 
> > > > Remove the work-around now.
> > > > 
> > > > Signed-off-by: Stefano Stabellini 
> > > > CC: Christopher Clark 
> > > 
> > > Presumably you tried to cross-compile it for Arm? It would be good to
> > > mention that in the commit message.
> > > 
> > > I think the content of this patch is fine:
> > 
> > It cross-compiles fine for aarch64 on x86_64 with Yocto.  Although we
> > don't do any cross-compilations in OSSTest as far as I know, so applying
> > the patch won't break OSSTest, given the state of the release, I think
> > it would be best to wait for the next merge window.
> 
> That's of course fine by me.

I'll resend the patch with a better commit message. It would be good to
get an ack if you are OK with it (of course there will be no committing
it right now.)

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

[Xen-devel] [linux-4.19 test] 143841: regressions - FAIL

2019-11-07 Thread osstest service owner
flight 143841 linux-4.19 real [real]
http://logs.test-lab.xenproject.org/osstest/logs/143841/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qcow2   19 guest-start/debian.repeat fail REGR. vs. 142932
 test-amd64-amd64-libvirt-vhd 17 guest-start/debian.repeat fail REGR. vs. 142932
 test-amd64-i386-xl-raw  19 guest-start/debian.repeat fail REGR. vs. 142932
 test-armhf-armhf-libvirt-raw 15 guest-start/debian.repeat fail REGR. vs. 142932

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail in 143600 pass 
in 143841
 test-amd64-i386-qemuu-rhel6hvm-intel 12 guest-start/redhat.repeat fail pass in 
143600

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeat fail in 143600 like 
142880
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail like 142932
 test-armhf-armhf-xl-vhd  15 guest-start/debian.repeatfail  like 142932
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 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-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-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 

[Xen-devel] [OSSTEST PATCH] make-flight: Drop all win10 tests in all flights

2019-11-07 Thread Ian Jackson
These are failing and have been for some time and it does not appear
that anyone has the capability to fix them.  Running them in these
circumstances seems wasteful.

Effect is to drop test-*-win10-* jobs (checked with
standalone-generate-dump-flight-runvars).

CC: Sander Eikelenboom 
CC: Jürgen Groß 
Signed-off-by: Ian Jackson 
---
 make-flight | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/make-flight b/make-flight
index be620c6d..b08431dc 100755
--- a/make-flight
+++ b/make-flight
@@ -327,7 +327,7 @@ do_hvm_win7_x64_tests () {
 
 do_hvm_win_2017_tests () {
   do_hvm_win_test_one ws16  ws16   amd64 guests_memsize=3584
-  do_hvm_win_test_one win10 win10v1703 i386  guests_memsize=3584
+# do_hvm_win_test_one win10 win10v1703 i386  guests_memsize=3584
 }
 
 do_hvm_debian_nested_tests () {
-- 
2.11.0


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

Re: [Xen-devel] OSStest priorities

2019-11-07 Thread Sander Eikelenboom
On 07/11/2019 17:44, Jürgen Groß wrote:
> Hi Ian,
> 
> in the Xen community call we agreed to try to speed up OSStest for
> xen-unstable in order to make better progress with the 4.13 release.
> 
> Could you please suspend testing for Xen 4.10 and older (Jan agreed on
> that), and disable the Linux kernel tests which are currently failing
> (including the bisecting)?
> 
> This should free lots of resources in OSStest reducing xen-unstable
> test latencies.
> 
> 
> Juergen
> 
> 

The following tests have quite a long timeout and will always fail,
(although last time I manually checked windows 10 actually installed fine
on a HVM on my machine).

 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass

perhaps suspending these in all tree until the underlying install image is fixed
or replaced, would also free some more resources, also after the release.

--
Sander

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

[Xen-devel] [PATCH v2] tools/hotpug: only attempt to call 'ip route' if there is valid command

2019-11-07 Thread paul
From: Paul Durrant 

The vif-route script should only call 'ip route' when 'ipcmd' has been
set, otherwise it will fail due to an incorrect command string.

This patch also adds routes for 'tap' (i.e. emulated) devices as well as
'vif' (i.e. PV) devices by making use of the route metric. Emulated
devices are used by HVM guests until they are unplugged, at which point the
PV device becomes active. Thus 'tap' devices should get a higher priority
(i.e. lower numbered) metric than 'vif' devices.

There is also one small whitespace fix.

NOTE: Empirically offline/online commands relate to 'vif' devices, and
  add/remove commands relate to 'tap' devices. However, this patch
  treats them equally and uses ${type_if} to distinguish.

Signed-off-by: Paul Durrant 
---
Cc: Ian Jackson 
Cc: Wei Liu 
---
 tools/hotplug/Linux/vif-route | 22 +++---
 1 file changed, 19 insertions(+), 3 deletions(-)
 mode change 100644 => 100755 tools/hotplug/Linux/vif-route

diff --git a/tools/hotplug/Linux/vif-route b/tools/hotplug/Linux/vif-route
old mode 100644
new mode 100755
index c149ffc..e71acae
--- a/tools/hotplug/Linux/vif-route
+++ b/tools/hotplug/Linux/vif-route
@@ -22,12 +22,16 @@ dir=$(dirname "$0")
 main_ip=$(dom0_ip)
 
 case "${command}" in
+add)
+;&
 online)
 ifconfig ${dev} ${main_ip} netmask 255.255.255.255 up
 echo 1 >/proc/sys/net/ipv4/conf/${dev}/proxy_arp
 ipcmd='add'
 cmdprefix=''
 ;;
+remove)
+;&
 offline)
 do_without_error ifdown ${dev}
 ipcmd='del'
@@ -35,11 +39,23 @@ case "${command}" in
 ;;
 esac
 
-if [ "${ip}" ] ; then
+case "${type_if}" in
+tap)
+   metric=1
+   ;;
+vif)
+   metric=2
+   ;;
+*)
+   fatal "Unrecognised interface type ${type_if}"
+   ;;
+esac
+
+if [ "${ipcmd}" ] ; then
 # If we've been given a list of IP addresses, then add routes from dom0 to
 # the guest using those addresses.
 for addr in ${ip} ; do
-  ${cmdprefix} ip route ${ipcmd} ${addr} dev ${dev} src ${main_ip}
+  ${cmdprefix} ip route ${ipcmd} ${addr} dev ${dev} src ${main_ip} metric 
${metric}
 done
 fi
 
@@ -50,5 +66,5 @@ call_hooks vif post
 log debug "Successful vif-route ${command} for ${dev}."
 if [ "${command}" = "online" ]
 then
-  success
+success
 fi
-- 
2.7.4


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

Re: [Xen-devel] OSStest priorities

2019-11-07 Thread Ian Jackson
Jürgen Groß writes ("OSStest priorities"):
> in the Xen community call we agreed to try to speed up OSStest for
> xen-unstable in order to make better progress with the 4.13 release.
> 
> Could you please suspend testing for Xen 4.10 and older (Jan agreed on
> that), and disable the Linux kernel tests which are currently failing
> (including the bisecting)?
> 
> This should free lots of resources in OSStest reducing xen-unstable
> test latencies.

No problem.  Done.  (I didn't cancel the in-progress flights, so a few
reports will still appear for these suspended branches.)

I will keep an eye on it.

Regards,
Ian.

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

[Xen-devel] [linux-4.9 test] 143825: regressions - FAIL

2019-11-07 Thread osstest service owner
flight 143825 linux-4.9 real [real]
http://logs.test-lab.xenproject.org/osstest/logs/143825/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-libvirt-vhd 17 guest-start/debian.repeat fail REGR. vs. 142947
 test-amd64-i386-xl-raw  19 guest-start/debian.repeat fail REGR. vs. 142947
 test-amd64-amd64-xl-qcow2   19 guest-start/debian.repeat fail REGR. vs. 142947
 test-armhf-armhf-xl-vhd 15 guest-start/debian.repeat fail REGR. vs. 142947
 test-armhf-armhf-libvirt-raw 15 guest-start/debian.repeat fail REGR. vs. 142947

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-amd64-pvgrub  7 xen-bootfail in 143526 pass in 143825
 test-amd64-amd64-xl-pvshim 20 guest-start/debian.repeat fail in 143630 pass in 
143418
 test-armhf-armhf-xl-rtds 15 guest-stop   fail in 143630 pass in 143526
 test-amd64-amd64-xl-rtds15 guest-saverestore fail in 143630 pass in 143825
 test-amd64-amd64-xl-pvshim   16 guest-localmigrate fail pass in 143630
 test-armhf-armhf-xl-rtds 12 guest-startfail pass in 143630

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-rtds  18 guest-localmigrate/x10 fail in 143418 like 142947
 test-armhf-armhf-xl-rtds13 migrate-support-check fail in 143630 never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-check fail in 143630 never pass
 test-amd64-amd64-xl-rtds 16 guest-localmigrate   fail  like 142893
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 142947
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 142947
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 142947
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 142947
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 142947
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail never pass
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail never pass
 test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass
 test-amd64-amd64-xl-pvhv2-amd 12 guest-start  fail  never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 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-i386-xl-qemut-win10-i386 10 windows-install fail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail 

[Xen-devel] OSStest priorities

2019-11-07 Thread Jürgen Groß

Hi Ian,

in the Xen community call we agreed to try to speed up OSStest for
xen-unstable in order to make better progress with the 4.13 release.

Could you please suspend testing for Xen 4.10 and older (Jan agreed on
that), and disable the Linux kernel tests which are currently failing
(including the bisecting)?

This should free lots of resources in OSStest reducing xen-unstable
test latencies.


Juergen

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

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

2019-11-07 Thread osstest service owner
flight 143753 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/143753/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-pvshim   18 guest-localmigrate/x10   fail REGR. vs. 142915
 test-amd64-amd64-xl-qcow2   19 guest-start/debian.repeat fail REGR. vs. 142915
 test-amd64-i386-xl-raw  19 guest-start/debian.repeat fail REGR. vs. 142915
 test-amd64-amd64-libvirt-vhd 17 guest-start/debian.repeat fail REGR. vs. 142915
 test-armhf-armhf-libvirt-raw 15 guest-start/debian.repeat fail REGR. vs. 142915
 test-armhf-armhf-xl-vhd 15 guest-start/debian.repeat fail REGR. vs. 142915

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-rtds 16 guest-localmigrate   fail REGR. vs. 142915

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

version targeted for testing:
 qemuu36609b4fa36f0ac934874371874416f7533a5408

Re: [Xen-devel] [PATCH for-4.13 2/2] x86/ioapic: don't use raw entry reads/writes in clear_IO_APIC_pin

2019-11-07 Thread Jan Beulich
On 07.11.2019 16:46, Roger Pau Monné  wrote:
> On Thu, Nov 07, 2019 at 04:28:56PM +0100, Jan Beulich wrote:
>> On 07.11.2019 16:06, Roger Pau Monne wrote:
>>> @@ -530,9 +530,9 @@ static void clear_IO_APIC_pin(unsigned int apic, 
>>> unsigned int pin)
>>>   */
>>>  memset(, 0, sizeof(entry));
>>>  entry.mask = 1;
>>> -__ioapic_write_entry(apic, pin, true, entry);
>>> +__ioapic_write_entry(apic, pin, false, entry);
>>
>> I may be able to understand why this one can't use raw mode, but as
>> per above a better overall description is needed.
> 
> Yes, this is the one that's actually incorrect, but see my reasoning
> below.
> 
>>
>>> -entry = __ioapic_read_entry(apic, pin, true);
>>> +entry = __ioapic_read_entry(apic, pin, false);
>>>  if (entry.irr)
>>>  printk(KERN_ERR "IO-APIC%02x-%u: Unable to reset IRR\n",
>>> IO_APIC_ID(apic), pin);
>>
>> This read again shouldn't need conversion, as the IRR bit doesn't
>> get touched (I think) by the interrupt remapping code during the
>> translation it does.
> 
> TBH, I think raw mode should only be used by the iommu code in order
> to setup the entries to point to the interrupt remapping table,
> everything else shouldn't be using raw mode. While it's true that some
> of the cases here are safe to use raw mode I would discourage it's
> usage as it can lead to issues, and this is not a performance critical
> path anyway.

You also should take the other possible perspective - not using
raw mode means going through interrupt remapping logic, which
can (needlessly) trigger errors. I think you want to break the
patch into a necessary and an optional part. The optional part
should be discussed separately and deferred until after 4.13.

Jan

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

Re: [Xen-devel] [PATCH for-4.13 2/2] x86/ioapic: don't use raw entry reads/writes in clear_IO_APIC_pin

2019-11-07 Thread Roger Pau Monné
On Thu, Nov 07, 2019 at 04:46:32PM +0100, Roger Pau Monné wrote:
> On Thu, Nov 07, 2019 at 04:28:56PM +0100, Jan Beulich wrote:
> > On 07.11.2019 16:06, Roger Pau Monne wrote:
> > > clear_IO_APIC_pin can be called after the iommu has been enabled, and
> > > using raw entry reads and writes will result in a misconfiguration of
> > > the entries already setup to use the interrupt remapping table.
> > 
> > I'm afraid I don't understand this: Raw reads and writes don't even
> > go to the IOMMU interrupt remapping code, so how would the assertion
> > be triggered?
> 
> Because the code does something like:
> 
> memset(, 0, ...);
> ...
> __ioapic_write_entry(apic, pin, true, rte);
> 
> At which point you misconfigure an ioapic entry that was already setup
> to point to an interrupt remapping entry, and the AMD IOMMU code
> chokes in the assert below.

Just to clarify since I think my reply hasn't been fully clear, the
ASSERT doesn't trigger in clear_IO_APIC_pin, but at a later point when
the IO-APIC entry is configured.

Thanks, Roger.

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

[Xen-devel] [linux-4.14 test] 143834: regressions - FAIL

2019-11-07 Thread osstest service owner
flight 143834 linux-4.14 real [real]
http://logs.test-lab.xenproject.org/osstest/logs/143834/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qcow2   19 guest-start/debian.repeat fail REGR. vs. 142849
 test-amd64-amd64-libvirt-vhd 17 guest-start/debian.repeat fail REGR. vs. 142849
 test-amd64-i386-xl-raw  19 guest-start/debian.repeat fail REGR. vs. 142849
 test-armhf-armhf-xl-vhd 15 guest-start/debian.repeat fail REGR. vs. 142849
 test-armhf-armhf-libvirt-raw 15 guest-start/debian.repeat fail REGR. vs. 142849

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-examine 11 examine-serial/bootloaderfail  like 142849
 test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10   fail  like 142849
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail  like 142849
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-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-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 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-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass

Re: [Xen-devel] [PATCH for-4.13 2/2] x86/ioapic: don't use raw entry reads/writes in clear_IO_APIC_pin

2019-11-07 Thread Roger Pau Monné
On Thu, Nov 07, 2019 at 04:28:56PM +0100, Jan Beulich wrote:
> On 07.11.2019 16:06, Roger Pau Monne wrote:
> > clear_IO_APIC_pin can be called after the iommu has been enabled, and
> > using raw entry reads and writes will result in a misconfiguration of
> > the entries already setup to use the interrupt remapping table.
> 
> I'm afraid I don't understand this: Raw reads and writes don't even
> go to the IOMMU interrupt remapping code, so how would the assertion
> be triggered?

Because the code does something like:

memset(, 0, ...);
...
__ioapic_write_entry(apic, pin, true, rte);

At which point you misconfigure an ioapic entry that was already setup
to point to an interrupt remapping entry, and the AMD IOMMU code
chokes in the assert below.

> 
> > (XEN) [   10.082154] ENABLING IO-APIC IRQs
> > (XEN) [   10.087789]  -> Using new ACK method
> > (XEN) [   10.093738] Assertion 'get_rte_index(rte) == offset' failed at 
> > iommu_intr.c:328
> > 
> > Signed-off-by: Roger Pau Monné 
> 
> "Reported-by: Sergey ..." ahead of this?

Oh yes.

> > --- a/xen/arch/x86/io_apic.c
> > +++ b/xen/arch/x86/io_apic.c
> > @@ -514,13 +514,13 @@ static void clear_IO_APIC_pin(unsigned int apic, 
> > unsigned int pin)
> >  entry.mask = 1;
> >  __ioapic_write_entry(apic, pin, false, entry);
> >  }
> > -entry = __ioapic_read_entry(apic, pin, true);
> > +entry = __ioapic_read_entry(apic, pin, false);
> >  
> >  if (entry.irr) {
> >  /* Make sure the trigger mode is set to level. */
> >  if (!entry.trigger) {
> >  entry.trigger = 1;
> > -__ioapic_write_entry(apic, pin, true, entry);
> > +__ioapic_write_entry(apic, pin, false, entry);
> >  }
> 
> All we do here is set the trigger bit. No translation back and forth
> of the RTE should be needed.
> 
> > @@ -530,9 +530,9 @@ static void clear_IO_APIC_pin(unsigned int apic, 
> > unsigned int pin)
> >   */
> >  memset(, 0, sizeof(entry));
> >  entry.mask = 1;
> > -__ioapic_write_entry(apic, pin, true, entry);
> > +__ioapic_write_entry(apic, pin, false, entry);
> 
> I may be able to understand why this one can't use raw mode, but as
> per above a better overall description is needed.

Yes, this is the one that's actually incorrect, but see my reasoning
below.

> 
> > -entry = __ioapic_read_entry(apic, pin, true);
> > +entry = __ioapic_read_entry(apic, pin, false);
> >  if (entry.irr)
> >  printk(KERN_ERR "IO-APIC%02x-%u: Unable to reset IRR\n",
> > IO_APIC_ID(apic), pin);
> 
> This read again shouldn't need conversion, as the IRR bit doesn't
> get touched (I think) by the interrupt remapping code during the
> translation it does.

TBH, I think raw mode should only be used by the iommu code in order
to setup the entries to point to the interrupt remapping table,
everything else shouldn't be using raw mode. While it's true that some
of the cases here are safe to use raw mode I would discourage it's
usage as it can lead to issues, and this is not a performance critical
path anyway.

Thanks, Roger.

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

Re: [Xen-devel] [PATCH] tools: pygrub actually cross-compiles just fine

2019-11-07 Thread Wei Liu
On Wed, Nov 06, 2019 at 08:10:47AM -0800, Stefano Stabellini wrote:
> On Wed, 6 Nov 2019, Wei Liu wrote:
> > On Tue, Nov 05, 2019 at 03:51:13PM -0800, Stefano Stabellini wrote:
> > > Actually, pygrub cross-compiles without issues. The cross-compilation
> > > work-around goes back to 2005 and it probably referred to PowerPC.
> > > 
> > > Remove the work-around now.
> > > 
> > > Signed-off-by: Stefano Stabellini 
> > > CC: Christopher Clark 
> > 
> > Presumably you tried to cross-compile it for Arm? It would be good to
> > mention that in the commit message.
> > 
> > I think the content of this patch is fine:
> 
> It cross-compiles fine for aarch64 on x86_64 with Yocto.  Although we
> don't do any cross-compilations in OSSTest as far as I know, so applying
> the patch won't break OSSTest, given the state of the release, I think
> it would be best to wait for the next merge window.

That's of course fine by me.

Wei.

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

Re: [Xen-devel] [PATCH v1 04/10] vfio/type1: Prepare is_invalid_reserved_pfn() for PG_reserved changes

2019-11-07 Thread Dan Williams
On Thu, Oct 24, 2019 at 5:12 AM David Hildenbrand  wrote:
>
> Right now, ZONE_DEVICE memory is always set PG_reserved. We want to
> change that.
>
> KVM has this weird use case that you can map anything from /dev/mem
> into the guest. pfn_valid() is not a reliable check whether the memmap
> was initialized and can be touched. pfn_to_online_page() makes sure
> that we have an initialized memmap (and don't have ZONE_DEVICE memory).
>
> Rewrite is_invalid_reserved_pfn() similar to kvm_is_reserved_pfn() to make
> sure the function produces the same result once we stop setting ZONE_DEVICE
> pages PG_reserved.
>
> Cc: Alex Williamson 
> Cc: Cornelia Huck 
> Signed-off-by: David Hildenbrand 
> ---
>  drivers/vfio/vfio_iommu_type1.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 2ada8e6cdb88..f8ce8c408ba8 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -299,9 +299,15 @@ static int vfio_lock_acct(struct vfio_dma *dma, long 
> npage, bool async)
>   */
>  static bool is_invalid_reserved_pfn(unsigned long pfn)
>  {
> -   if (pfn_valid(pfn))
> -   return PageReserved(pfn_to_page(pfn));
> +   struct page *page = pfn_to_online_page(pfn);

Ugh, I just realized this is not a safe conversion until
pfn_to_online_page() is moved over to subsection granularity. As it
stands it will return true for any ZONE_DEVICE pages that share a
section with boot memory.

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

Re: [Xen-devel] [PATCH for-4.13 2/2] x86/ioapic: don't use raw entry reads/writes in clear_IO_APIC_pin

2019-11-07 Thread Jan Beulich
On 07.11.2019 16:06, Roger Pau Monne wrote:
> clear_IO_APIC_pin can be called after the iommu has been enabled, and
> using raw entry reads and writes will result in a misconfiguration of
> the entries already setup to use the interrupt remapping table.

I'm afraid I don't understand this: Raw reads and writes don't even
go to the IOMMU interrupt remapping code, so how would the assertion
be triggered?

> (XEN) [   10.082154] ENABLING IO-APIC IRQs
> (XEN) [   10.087789]  -> Using new ACK method
> (XEN) [   10.093738] Assertion 'get_rte_index(rte) == offset' failed at 
> iommu_intr.c:328
> 
> Signed-off-by: Roger Pau Monné 

"Reported-by: Sergey ..." ahead of this?

> --- a/xen/arch/x86/io_apic.c
> +++ b/xen/arch/x86/io_apic.c
> @@ -514,13 +514,13 @@ static void clear_IO_APIC_pin(unsigned int apic, 
> unsigned int pin)
>  entry.mask = 1;
>  __ioapic_write_entry(apic, pin, false, entry);
>  }
> -entry = __ioapic_read_entry(apic, pin, true);
> +entry = __ioapic_read_entry(apic, pin, false);
>  
>  if (entry.irr) {
>  /* Make sure the trigger mode is set to level. */
>  if (!entry.trigger) {
>  entry.trigger = 1;
> -__ioapic_write_entry(apic, pin, true, entry);
> +__ioapic_write_entry(apic, pin, false, entry);
>  }

All we do here is set the trigger bit. No translation back and forth
of the RTE should be needed.

> @@ -530,9 +530,9 @@ static void clear_IO_APIC_pin(unsigned int apic, unsigned 
> int pin)
>   */
>  memset(, 0, sizeof(entry));
>  entry.mask = 1;
> -__ioapic_write_entry(apic, pin, true, entry);
> +__ioapic_write_entry(apic, pin, false, entry);

I may be able to understand why this one can't use raw mode, but as
per above a better overall description is needed.

> -entry = __ioapic_read_entry(apic, pin, true);
> +entry = __ioapic_read_entry(apic, pin, false);
>  if (entry.irr)
>  printk(KERN_ERR "IO-APIC%02x-%u: Unable to reset IRR\n",
> IO_APIC_ID(apic), pin);

This read again shouldn't need conversion, as the IRR bit doesn't
get touched (I think) by the interrupt remapping code during the
translation it does.

Jan

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

Re: [Xen-devel] [PATCH for-4.13 1/2] x86/ioapic: remove usage of TRUE and FALSE in clear_IO_APIC_pin

2019-11-07 Thread Jan Beulich
On 07.11.2019 16:06, Roger Pau Monne wrote:
> And instead use proper booleans. No functional change intended.
> 
> Signed-off-by: Roger Pau Monné 

Other than Andrew I think this is fine without further extension,
i.e.
Acked-by: Jan Beulich 
Of course this isn't to say that I wouldn't welcome a more complete
cleanup, as suggested by him.

Jan

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

Re: [Xen-devel] [PATCH for-4.13 v2] passthrough: simplify locking and logging

2019-11-07 Thread Jan Beulich
On 01.11.2019 19:28, Igor Druzhinin wrote:
> This patch instead acquires the lock once for assignment (or test assign)
> operations directly in iommu_do_pci_domctl() and thus can remove the
> duplicate domain ownership check in assign_device(). Whilst in the
> neighbourhood, the patch also removes some debug logging from
> assign_device() and deassign_device() and replaces it with proper error
> logging, which allows error logging in iommu_do_pci_domctl() to be
> removed. Also, since device_assigned() can tell the difference between a
> guest assigned device and a non-existent one, log the actual error
> condition rather then being ambiguous for the sake a few extra lines of
> code.

In this last sentence it looks like you mean the caller of
device_assigned(), not the function itself.

> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -932,30 +932,27 @@ static int deassign_device(struct domain *d, uint16_t 
> seg, uint8_t bus,
>  break;
>  ret = hd->platform_ops->reassign_device(d, target, devfn,
>  pci_to_dev(pdev));
> -if ( !ret )
> -continue;
> -
> -printk(XENLOG_G_ERR "%pd: deassign %04x:%02x:%02x.%u failed (%d)\n",
> -   d, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), ret);
> -return ret;
> +if ( ret )
> +goto out;
>  }
>  
>  devfn = pdev->devfn;
>  ret = hd->platform_ops->reassign_device(d, target, devfn,
>  pci_to_dev(pdev));
>  if ( ret )
> -{
> -dprintk(XENLOG_G_ERR,
> -"%pd: deassign device (%04x:%02x:%02x.%u) failed\n",
> -d, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
> -return ret;
> -}
> +goto out;
>  
>  if ( pdev->domain == hardware_domain  )
>  pdev->quarantine = false;
>  
>  pdev->fault.count = 0;
>  
> +out:
> +if ( ret )
> +printk(XENLOG_G_ERR
> +   "%pd: deassign device (%04x:%02x:%02x.%u) failed (%d)\n", d,
> +   seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), ret);
> +
>  return ret;
>  }

There would have been quite a bit less code churn if you simply
replaced the dprintk(). If you really want to stick to the goto
approach you're introducing (which I dislike, but I know others
prefer it in cases like this one), then please indent the label and
shorten the message to the one that was used in the original printk().

> @@ -1549,11 +1542,10 @@ static int device_assigned(u16 seg, u8 bus, u8 devfn)
>pdev->domain != dom_io )
>  rc = -EBUSY;
>  
> -pcidevs_unlock();
> -
>  return rc;
>  }
>  
> +/* caller should hold the pcidevs_lock */
>  static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 
> flag)

Just like the comment ahead of deassign_device(), this one should
start with a capital letter.

> @@ -1608,19 +1588,17 @@ static int assign_device(struct domain *d, u16 seg, 
> u8 bus, u8 devfn, u32 flag)
>  if ( PCI_SLOT(devfn) != PCI_SLOT(pdev->devfn) )
>  break;
>  rc = hd->platform_ops->assign_device(d, devfn, pci_to_dev(pdev), 
> flag);
> -if ( rc )
> -printk(XENLOG_G_WARNING "d%d: assign %04x:%02x:%02x.%u failed 
> (%d)\n",
> -   d->domain_id, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
> -   rc);
>  }
>  
>   done:
> +if ( rc )
> +printk(XENLOG_G_ERR
> +   "%pd: assign device (%04x:%02x:%02x.%u) failed (%d)\n", d,
> +   seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), rc);

I'd prefer if we could stick to this being XENLOG_G_WARNING: Other
than device de-assignment failure, assignment failure is unlikely
to be an issue to the host as a whole. Also please again shorten
the message to what it was before.

> @@ -1776,29 +1754,40 @@ int iommu_do_pci_domctl(
>  bus = PCI_BUS(machine_sbdf);
>  devfn = PCI_DEVFN2(machine_sbdf);
>  
> +pcidevs_lock();
>  ret = device_assigned(seg, bus, devfn);
>  if ( domctl->cmd == XEN_DOMCTL_test_assign_device )
>  {
> -if ( ret )
> +switch ( ret )
>  {
> +case 0:
> +break;
> +
> +case -ENODEV:
>  printk(XENLOG_G_INFO
> -   "%04x:%02x:%02x.%u already assigned, or 
> non-existent\n",
> +   "%04x:%02x:%02x.%u non-existent\n",
> seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
>  ret = -EINVAL;
> +break;
> +
> +case -EBUSY:
> +printk(XENLOG_G_INFO
> +   "%04x:%02x:%02x.%u already assigned\n",
> +   seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
> +ret = -EINVAL;
> +break;
> +
> +default:
> +ret = -EINVAL;
> 

Re: [Xen-devel] [PATCH for-4.13 1/2] x86/ioapic: remove usage of TRUE and FALSE in clear_IO_APIC_pin

2019-11-07 Thread Andrew Cooper
On 07/11/2019 15:06, Roger Pau Monne wrote:
> And instead use proper booleans. No functional change intended.
>
> Signed-off-by: Roger Pau Monné 
> ---
> Cc: Juergen Gross 
> ---

Urgh.  Can we purge with prejudice all of this TRUE and FALSE nonsense? 
There are only a few users more than this path.

EFI adds in conditionally in a buggy way (which I thought I'd properly
excised back with the switch from bool_t to bool, but clearly not), and
ACPI undef's its surrounding and implements it differently.

Code like this has no buisness remaining, and what we currently have is
a disaster waiting to happen.

~Andrew

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

[Xen-devel] [PATCH for-4.13 2/2] x86/ioapic: don't use raw entry reads/writes in clear_IO_APIC_pin

2019-11-07 Thread Roger Pau Monne
clear_IO_APIC_pin can be called after the iommu has been enabled, and
using raw entry reads and writes will result in a misconfiguration of
the entries already setup to use the interrupt remapping table. This
fixes the following panic seen on AMD Rome boxes:

(XEN) [   10.082154] ENABLING IO-APIC IRQs
(XEN) [   10.087789]  -> Using new ACK method
(XEN) [   10.093738] Assertion 'get_rte_index(rte) == offset' failed at 
iommu_intr.c:328

Signed-off-by: Roger Pau Monné 
---
Cc: Juergen Gross 
---
 xen/arch/x86/io_apic.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index b9c66acdb3..13b41b46a3 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -514,13 +514,13 @@ static void clear_IO_APIC_pin(unsigned int apic, unsigned 
int pin)
 entry.mask = 1;
 __ioapic_write_entry(apic, pin, false, entry);
 }
-entry = __ioapic_read_entry(apic, pin, true);
+entry = __ioapic_read_entry(apic, pin, false);
 
 if (entry.irr) {
 /* Make sure the trigger mode is set to level. */
 if (!entry.trigger) {
 entry.trigger = 1;
-__ioapic_write_entry(apic, pin, true, entry);
+__ioapic_write_entry(apic, pin, false, entry);
 }
 __io_apic_eoi(apic, entry.vector, pin);
 }
@@ -530,9 +530,9 @@ static void clear_IO_APIC_pin(unsigned int apic, unsigned 
int pin)
  */
 memset(, 0, sizeof(entry));
 entry.mask = 1;
-__ioapic_write_entry(apic, pin, true, entry);
+__ioapic_write_entry(apic, pin, false, entry);
 
-entry = __ioapic_read_entry(apic, pin, true);
+entry = __ioapic_read_entry(apic, pin, false);
 if (entry.irr)
 printk(KERN_ERR "IO-APIC%02x-%u: Unable to reset IRR\n",
IO_APIC_ID(apic), pin);
-- 
2.23.0


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

[Xen-devel] [PATCH for-4.13 0/2] x86/ioapic: fix clear_IO_APIC_pin when using interrupt remapping

2019-11-07 Thread Roger Pau Monne
Hello,

Current code in clear_IO_APIC_pin doesn't properly deal with IO-APIC
entries already configured to point to entries in the iommu interrupt
remapping table, fix this.

Roger Pau Monne (2):
  x86/ioapic: remove usage of TRUE and FALSE in clear_IO_APIC_pin
  x86/ioapic: don't use raw entry reads/writes in clear_IO_APIC_pin

 xen/arch/x86/io_apic.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

-- 
2.23.0


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

[Xen-devel] [PATCH for-4.13 1/2] x86/ioapic: remove usage of TRUE and FALSE in clear_IO_APIC_pin

2019-11-07 Thread Roger Pau Monne
And instead use proper booleans. No functional change intended.

Signed-off-by: Roger Pau Monné 
---
Cc: Juergen Gross 
---
 xen/arch/x86/io_apic.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index 37eabc16c9..b9c66acdb3 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -502,7 +502,7 @@ static void clear_IO_APIC_pin(unsigned int apic, unsigned 
int pin)
 struct IO_APIC_route_entry entry;
 
 /* Check delivery_mode to be sure we're not clearing an SMI pin */
-entry = __ioapic_read_entry(apic, pin, FALSE);
+entry = __ioapic_read_entry(apic, pin, false);
 if (entry.delivery_mode == dest_SMI)
 return;
 
@@ -512,15 +512,15 @@ static void clear_IO_APIC_pin(unsigned int apic, unsigned 
int pin)
  */
 if (!entry.mask) {
 entry.mask = 1;
-__ioapic_write_entry(apic, pin, FALSE, entry);
+__ioapic_write_entry(apic, pin, false, entry);
 }
-entry = __ioapic_read_entry(apic, pin, TRUE);
+entry = __ioapic_read_entry(apic, pin, true);
 
 if (entry.irr) {
 /* Make sure the trigger mode is set to level. */
 if (!entry.trigger) {
 entry.trigger = 1;
-__ioapic_write_entry(apic, pin, TRUE, entry);
+__ioapic_write_entry(apic, pin, true, entry);
 }
 __io_apic_eoi(apic, entry.vector, pin);
 }
@@ -530,9 +530,9 @@ static void clear_IO_APIC_pin(unsigned int apic, unsigned 
int pin)
  */
 memset(, 0, sizeof(entry));
 entry.mask = 1;
-__ioapic_write_entry(apic, pin, TRUE, entry);
+__ioapic_write_entry(apic, pin, true, entry);
 
-entry = __ioapic_read_entry(apic, pin, TRUE);
+entry = __ioapic_read_entry(apic, pin, true);
 if (entry.irr)
 printk(KERN_ERR "IO-APIC%02x-%u: Unable to reset IRR\n",
IO_APIC_ID(apic), pin);
-- 
2.23.0


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

Re: [Xen-devel] [PATCH V2 1/2] x86/altp2m: Add hypercall to set a range of sve bits

2019-11-07 Thread Tamas K Lengyel
On Wed, Nov 6, 2019 at 11:46 PM Alexandru Stefan ISAILA
 wrote:
>
>
>
> On 06.11.2019 23:06, Tamas K Lengyel wrote:
> > On Wed, Nov 6, 2019 at 7:35 AM Alexandru Stefan ISAILA
> >  wrote:
> >>
> >> By default the sve bits are not set.
> >> This patch adds a new hypercall, xc_altp2m_set_supress_ve_multi(),
> >> to set a range of sve bits.
> >> The core function, p2m_set_suppress_ve_multi(), does not brake in case
> >> of a error and it is doing a best effort for setting the bits in the
> >> given range. A check for continuation is made in order to have
> >> preemption on big ranges.
> >>
> >> Signed-off-by: Alexandru Isaila 
> >>
> >> ---
> >> Changes since V1:
> >>  - Remove "continue"
> >>  - Add a new field in xen_hvm_altp2m_suppress_ve to store the
> >> continuation value
> >>  - Have p2m_set_suppress_ve_multi() take
> >> xen_hvm_altp2m_suppress_ve as a param.
> >> ---
> >>   tools/libxc/include/xenctrl.h   |  3 ++
> >>   tools/libxc/xc_altp2m.c | 25 ++
> >>   xen/arch/x86/hvm/hvm.c  | 20 ++--
> >>   xen/arch/x86/mm/p2m.c   | 58 +
> >>   xen/include/public/hvm/hvm_op.h |  5 ++-
> >>   xen/include/xen/mem_access.h|  3 ++
> >>   6 files changed, 111 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> >> index f4431687b3..21b644f459 100644
> >> --- a/tools/libxc/include/xenctrl.h
> >> +++ b/tools/libxc/include/xenctrl.h
> >> @@ -1923,6 +1923,9 @@ int xc_altp2m_switch_to_view(xc_interface *handle, 
> >> uint32_t domid,
> >>uint16_t view_id);
> >>   int xc_altp2m_set_suppress_ve(xc_interface *handle, uint32_t domid,
> >> uint16_t view_id, xen_pfn_t gfn, bool sve);
> >> +int xc_altp2m_set_supress_ve_multi(xc_interface *handle, uint32_t domid,
> >> +   uint16_t view_id, xen_pfn_t start_gfn,
> >> +   uint32_t nr, bool sve);
> >>   int xc_altp2m_get_suppress_ve(xc_interface *handle, uint32_t domid,
> >> uint16_t view_id, xen_pfn_t gfn, bool 
> >> *sve);
> >>   int xc_altp2m_set_mem_access(xc_interface *handle, uint32_t domid,
> >> diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
> >> index 09dad0355e..6605d9abbe 100644
> >> --- a/tools/libxc/xc_altp2m.c
> >> +++ b/tools/libxc/xc_altp2m.c
> >> @@ -234,6 +234,31 @@ int xc_altp2m_set_suppress_ve(xc_interface *handle, 
> >> uint32_t domid,
> >>   return rc;
> >>   }
> >>
> >> +int xc_altp2m_set_supress_ve_multi(xc_interface *handle, uint32_t domid,
> >> +   uint16_t view_id, xen_pfn_t start_gfn,
> >> +   uint32_t nr, bool sve)
> >> +{
> >> +int rc;
> >> +DECLARE_HYPERCALL_BUFFER(xen_hvm_altp2m_op_t, arg);
> >> +
> >> +arg = xc_hypercall_buffer_alloc(handle, arg, sizeof(*arg));
> >
> > Does xc_hypercall_buffer_alloc null-initialize the structure?
> >
>
> It calls xencall_alloc_buffer_pages() which calls memset(p, 0, nr_pages
> * PAGE_SIZE) before returning.

Thanks!

Reviewed-by: Tamas K Lengyel 

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

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

2019-11-07 Thread osstest service owner
flight 143839 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/143839/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 1bcc65b9a1408cf445b7b3f9499b27d9c235db71
baseline version:
 ovmf 8d3f428109623096cb8845779cdf9dc44949b8e9

Last test of basis   143689  2019-11-04 06:37:00 Z3 days
Testing same since   143839  2019-11-05 15:38:49 Z1 days1 attempts


People who touched revisions under test:
  Laszlo Ersek 
  Ray Ni 
  Shenglei Zhang 

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



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/osstest/ovmf.git
   8d3f428109..1bcc65b9a1  1bcc65b9a1408cf445b7b3f9499b27d9c235db71 -> 
xen-tested-master

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

[Xen-devel] [xen-4.11-testing test] 143778: regressions - FAIL

2019-11-07 Thread osstest service owner
flight 143778 xen-4.11-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/143778/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qcow2   19 guest-start/debian.repeat fail REGR. vs. 143158
 test-amd64-amd64-libvirt-vhd 17 guest-start/debian.repeat fail REGR. vs. 143158
 test-amd64-i386-xl-raw  19 guest-start/debian.repeat fail REGR. vs. 143158
 test-armhf-armhf-xl-vhd 15 guest-start/debian.repeat fail REGR. vs. 143158
 test-armhf-armhf-libvirt-raw 15 guest-start/debian.repeat fail REGR. vs. 143158

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail never pass
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-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-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 

Re: [Xen-devel] IMPORTANT CORRECTION - Community Call: Call for Agenda Items and call details for Nov 7, CORRECTED TIME 16:00 - 17:00 UTC

2019-11-07 Thread Lars Kurth
Hi all,

I tripped over the fact that 
https://www.timeanddate.com/worldclock/meetingdetails.htm uses UTC as fixed 
point whereas my calendar uses the local time as fixed point, so the meeting 
mistakenly got pushed an hour earlier when DST finished. The correct time 
(which should correspond to your calendar entries) is

16:00 - 17:00 UTC
08:00 - 09:00 PST (San Francisco) - sorry for the early time slot. If 
this is a problem, let's discuss at the call
12:00 - 13:00 EST (New York)
16:00 - 17:00 FMT (London)
17:00 - 18:00 CET (Berlin)
23:00 - 01:00 CST (Beijing)
Further International meeting times: 
https://www.timeanddate.com/worldclock/meetingdetails.html?year=2018=11=7=16=0=0=224=24=179=136=37=33

Best Regards
Lars

On 07/11/2019, 07:24, "Lars Kurth"  wrote:

Hi all,

quick reminder re agenda items. What I have so far is at 
https://cryptpad.fr/pad/#/2/pad/edit/SkeU+Z5J9WIIU9ZsXlojiXcQ/ 

C.1) Any more 4.13 coordination (Juergen)
C.2) Volunteers/suggestions for Release Managers for 4.13 (Lars / Juergen)
C.3) 4.13 Release Notes / Blog Post / Feature List - needs review (Lars)
See 
https://docs.google.com/document/d/1EpigvxDzeoc1dOMFwQ9itDXY4vY7nvxPiLGcNQQmX28/edit?usp=sharing

AOB
1) Travel and discussions
Rich Persaud, Christopher Clark & Daniel Smith will be in Cambridge Dec 10 
pm & 11 am 
Discussions are planned around a number of topics such as state of XSM, 
DomB proposal as a secure means to start an L0/L1 configuration, KCONFIG for L0 
version of Xen, etc.
Citrix will host, but others are welcome to join (please contact Lars for 
logistics)

Feel free to request additional items

Regards
Lars

On 04/11/2019, 05:37, "Lars Kurth"  wrote:

Dear community members,
 
please send me agenda items for next week’s community call. A draft 
agenda is at https://cryptpad.fr/pad/#/2/pad/edit/SkeU+Z5J9WIIU9ZsXlojiXcQ/
Please add agenda items to the document or reply to this e-mail
Note that I am on PTO today and tomorrow
 
Last month’s minutes are at 
https://cryptpad.fr/pad/#/2/pad/edit/4FGEw81flPUiivkjkuvQJ-CK/
 
Best Regards
Lars

## Meeting time (please double check the times
15:00 - 16:00 UTC
07:00 - 08:00 PST (San Francisco) - sorry for the early time slot. If 
this is a problem, let's discuss at the call
10:00 - 11:00 EST (New York)
15:00 - 16:00 FMT (London)
16:00 - 17:00 CET (Berlin)
23:00 - 22:00 CST (Beijing)
Further International meeting times: 
https://www.timeanddate.com/worldclock/meetingdetails.html?year=2018=11=7=15=0=0=224=24=179=136=37=33

## Dial in details
Web: https://www.gotomeet.me/larskurth

You can also dial in using your phone.
Access Code: 906-886-965

China (Toll Free): 4008 811084
Germany: +49 692 5736 7317
Poland (Toll Free): 00 800 1124759
United Kingdom: +44 330 221 0088
United States: +1 (571) 317-3129

More phone numbers
Australia: +61 2 9087 3604
Austria: +43 7 2081 5427
Argentina (Toll Free): 0 800 444 3375
Bahrain (Toll Free): 800 81 111
Belarus (Toll Free): 8 820 0011 0400
Belgium: +32 28 93 7018
Brazil (Toll Free): 0 800 047 4906
Bulgaria (Toll Free): 00800 120 4417
Canada: +1 (647) 497-9391
Chile (Toll Free): 800 395 150
Colombia (Toll Free): 01 800 518 4483
Czech Republic (Toll Free): 800 500448
Denmark: +45 32 72 03 82
Finland: +358 923 17 0568
France: +33 170 950 594
Greece (Toll Free): 00 800 4414 3838
Hong Kong (Toll Free): 30713169
Hungary (Toll Free): (06) 80 986 255
Iceland (Toll Free): 800 7204
India (Toll Free): 18002669272
Indonesia (Toll Free): 007 803 020 5375
Ireland: +353 15 360 728
Israel (Toll Free): 1 809 454 830
Italy: +39 0 247 92 13 01
Japan (Toll Free): 0 120 663 800
Korea, Republic of (Toll Free): 00798 14 207 4914
Luxembourg (Toll Free): 800 85158
Malaysia (Toll Free): 1 800 81 6854
Mexico (Toll Free): 01 800 522 1133
Netherlands: +31 207 941 377
New Zealand: +64 9 280 6302
Norway: +47 21 93 37 51
Panama (Toll Free): 00 800 226 7928
Peru (Toll Free): 0 800 77023
Philippines (Toll Free): 1 800 1110 1661
Portugal (Toll Free): 800 819 575
Romania (Toll Free): 0 800 410 029
Russian Federation (Toll Free): 8 800 100 6203
Saudi Arabia (Toll Free): 800 844 3633
Singapore (Toll Free): 18007231323
South Africa (Toll Free): 0 800 555 447
Spain: +34 932 75 2004
Sweden: +46 853 527 827
Switzerland: +41 225 4599 

[Xen-devel] [libvirt test] 143789: regressions - FAIL

2019-11-07 Thread osstest service owner
flight 143789 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/143789/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail 
REGR. vs. 143023
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail 
REGR. vs. 143023
 test-amd64-i386-libvirt  12 guest-start  fail REGR. vs. 143023
 test-amd64-amd64-libvirt-pair 10 xen-boot/src_host   fail REGR. vs. 143023
 test-amd64-amd64-libvirt-vhd 10 debian-di-installfail REGR. vs. 143023
 test-amd64-amd64-libvirt-xsm 12 guest-start  fail REGR. vs. 143023
 test-arm64-arm64-libvirt-qcow2 10 debian-di-install  fail REGR. vs. 143023
 test-amd64-amd64-libvirt 12 guest-start  fail REGR. vs. 143023
 test-amd64-i386-libvirt-xsm  12 guest-start  fail REGR. vs. 143023
 test-amd64-i386-libvirt-pair 21 guest-start/debian   fail REGR. vs. 143023
 test-arm64-arm64-libvirt-xsm 12 guest-start  fail REGR. vs. 143023
 test-arm64-arm64-libvirt 12 guest-start  fail REGR. vs. 143023
 test-armhf-armhf-libvirt-raw 10 debian-di-installfail REGR. vs. 143023
 test-armhf-armhf-libvirt 12 guest-start  fail REGR. vs. 143023

version targeted for testing:
 libvirt  78a342441efca14680a934dc72d1b3d1ed9e8d3e
baseline version:
 libvirt  2cff65e4c60ed7b3c0c6a97d526d1f8d52c0e919

Last test of basis   143023  2019-10-22 04:19:26 Z   15 days
Failing since143051  2019-10-23 04:18:57 Z   14 days   11 attempts
Testing same since   143789  2019-11-05 01:46:59 Z1 days1 attempts


People who touched revisions under test:
  Andrea Bolognani 
  Eric Blake 
  Jim Fehlig 
  Ján Tomko 
  Maya Rashish 
  Michal Privoznik 
  Pavel Hrdina 
  Peter Krempa 

jobs:
 build-amd64-xsm  pass
 build-arm64-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   fail
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmfail
 test-amd64-amd64-libvirt-xsm fail
 test-arm64-arm64-libvirt-xsm fail
 test-amd64-i386-libvirt-xsm  fail
 test-amd64-amd64-libvirt fail
 test-arm64-arm64-libvirt fail
 test-armhf-armhf-libvirt fail
 test-amd64-i386-libvirt  fail
 test-amd64-amd64-libvirt-pairfail
 test-amd64-i386-libvirt-pair fail
 test-arm64-arm64-libvirt-qcow2   fail
 test-armhf-armhf-libvirt-raw fail
 test-amd64-amd64-libvirt-vhd fail



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

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

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

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


Not pushing.

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

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

Re: [Xen-devel] Community Call: Call for Agenda Items and call details for Nov 7, 15:00 - 16:00 UTC

2019-11-07 Thread Lars Kurth
Hi all,

quick reminder re agenda items. What I have so far is at 
https://cryptpad.fr/pad/#/2/pad/edit/SkeU+Z5J9WIIU9ZsXlojiXcQ/ 

C.1) Any more 4.13 coordination (Juergen)
C.2) Volunteers/suggestions for Release Managers for 4.13 (Lars / Juergen)
C.3) 4.13 Release Notes / Blog Post / Feature List - needs review (Lars)
See 
https://docs.google.com/document/d/1EpigvxDzeoc1dOMFwQ9itDXY4vY7nvxPiLGcNQQmX28/edit?usp=sharing

AOB
1) Travel and discussions
Rich Persaud, Christopher Clark & Daniel Smith will be in Cambridge Dec 10 pm & 
11 am 
Discussions are planned around a number of topics such as state of XSM, DomB 
proposal as a secure means to start an L0/L1 configuration, KCONFIG for L0 
version of Xen, etc.
Citrix will host, but others are welcome to join (please contact Lars for 
logistics)

Feel free to request additional items

Regards
Lars

On 04/11/2019, 05:37, "Lars Kurth"  wrote:

Dear community members,
 
please send me agenda items for next week’s community call. A draft agenda 
is at https://cryptpad.fr/pad/#/2/pad/edit/SkeU+Z5J9WIIU9ZsXlojiXcQ/
Please add agenda items to the document or reply to this e-mail
Note that I am on PTO today and tomorrow
 
Last month’s minutes are at 
https://cryptpad.fr/pad/#/2/pad/edit/4FGEw81flPUiivkjkuvQJ-CK/
 
Best Regards
Lars

## Meeting time (please double check the times
15:00 - 16:00 UTC
07:00 - 08:00 PST (San Francisco) - sorry for the early time slot. If this 
is a problem, let's discuss at the call
10:00 - 11:00 EST (New York)
15:00 - 16:00 FMT (London)
16:00 - 17:00 CET (Berlin)
23:00 - 22:00 CST (Beijing)
Further International meeting times: 
https://www.timeanddate.com/worldclock/meetingdetails.html?year=2018=11=7=15=0=0=224=24=179=136=37=33

## Dial in details
Web: https://www.gotomeet.me/larskurth

You can also dial in using your phone.
Access Code: 906-886-965

China (Toll Free): 4008 811084
Germany: +49 692 5736 7317
Poland (Toll Free): 00 800 1124759
United Kingdom: +44 330 221 0088
United States: +1 (571) 317-3129

More phone numbers
Australia: +61 2 9087 3604
Austria: +43 7 2081 5427
Argentina (Toll Free): 0 800 444 3375
Bahrain (Toll Free): 800 81 111
Belarus (Toll Free): 8 820 0011 0400
Belgium: +32 28 93 7018
Brazil (Toll Free): 0 800 047 4906
Bulgaria (Toll Free): 00800 120 4417
Canada: +1 (647) 497-9391
Chile (Toll Free): 800 395 150
Colombia (Toll Free): 01 800 518 4483
Czech Republic (Toll Free): 800 500448
Denmark: +45 32 72 03 82
Finland: +358 923 17 0568
France: +33 170 950 594
Greece (Toll Free): 00 800 4414 3838
Hong Kong (Toll Free): 30713169
Hungary (Toll Free): (06) 80 986 255
Iceland (Toll Free): 800 7204
India (Toll Free): 18002669272
Indonesia (Toll Free): 007 803 020 5375
Ireland: +353 15 360 728
Israel (Toll Free): 1 809 454 830
Italy: +39 0 247 92 13 01
Japan (Toll Free): 0 120 663 800
Korea, Republic of (Toll Free): 00798 14 207 4914
Luxembourg (Toll Free): 800 85158
Malaysia (Toll Free): 1 800 81 6854
Mexico (Toll Free): 01 800 522 1133
Netherlands: +31 207 941 377
New Zealand: +64 9 280 6302
Norway: +47 21 93 37 51
Panama (Toll Free): 00 800 226 7928
Peru (Toll Free): 0 800 77023
Philippines (Toll Free): 1 800 1110 1661
Portugal (Toll Free): 800 819 575
Romania (Toll Free): 0 800 410 029
Russian Federation (Toll Free): 8 800 100 6203
Saudi Arabia (Toll Free): 800 844 3633
Singapore (Toll Free): 18007231323
South Africa (Toll Free): 0 800 555 447
Spain: +34 932 75 2004
Sweden: +46 853 527 827
Switzerland: +41 225 4599 78
Taiwan (Toll Free): 0 800 666 854
Thailand (Toll Free): 001 800 011 023
Turkey (Toll Free): 00 800 4488 23683
Ukraine (Toll Free): 0 800 50 1733
United Arab Emirates (Toll Free): 800 044 40439
Uruguay (Toll Free): 0004 019 1018
Viet Nam (Toll Free): 122 80 481

First GoToMeeting? Let's do a quick system check:
https://link.gotomeeting.com/system-check




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

Re: [Xen-devel] [PATCH 0/3] AMD/IOMMU: re-work mode updating

2019-11-07 Thread Jan Beulich
On 07.11.2019 13:49, Andrew Cooper wrote:
> On 07/11/2019 07:36, Jan Beulich wrote:
>> On 06.11.2019 18:31, Andrew Cooper wrote:
>>> On 06/11/2019 15:16, Jan Beulich wrote:
 update_paging_mode() in the AMD IOMMU code expects to be invoked with
 the PCI devices lock held. The check occurring only when the mode
 actually needs updating, the violation of this rule by the majority
 of callers did go unnoticed until per-domain IOMMU setup was changed
 to do away with on-demand creation of IOMMU page tables.

 Unfortunately the only half way reasonable fix to this that I could
 come up with requires more re-work than would seem desirable at this
 time of the release process, but addressing the issue seems
 unavoidable to me as its manifestation is a regression from the
 IOMMU page table setup re-work. The change also isn't without risk
 of further regressions - if in patch 2 I've missed a code path that
 would also need to invoke the new hook, then this might mean non-
 working guests (with passed-through devices on AMD hardware).

 1: AMD/IOMMU: don't needlessly trigger errors/crashes when unmapping a page
 2: introduce GFN notification for translated domains
 3: AMD/IOMMU: use notify_dfn() hook to update paging mode
>>> Having now looked at all three, why don't we just delete the dynamic
>>> height of AMD IOMMU pagetables?
>>>
>>> This series looks suspiciously like it is adding new common
>>> infrastructure to work around the fact we're doing something fairly dumb
>>> to being with.
>>>
>>> Hardcoding at 4 levels is, at the very worst, 2 extra pages per domain,
>>> and a substantial reduction in the complexity of the IOMMU code.
>> Yet an additional level of page walks hardware has to perform. Also
>> 4 levels won't cover all possible 52 address bits. And finally, the
>> more applicable your "substantial reduction", the less suitable such
>> a change may be at this point of the release (but I didn't look at
>> this side of things in any detail, so it may well not be an issue).
> 
> There is, in practice, no such thing as an HVM guest using 2 levels. 
> The VRAM just below the 4G boundary will force a resize to 3 levels
> during domain construction, and as a 1-line fix for 4.13, this probably
> isn't the worst idea going.

So here (with the 1-line fix remark) you talk about 3 levels. Yet
switching the 2 that we start from to 3 won't fix anything, as we
still may need to go to 4 for huge guests. Such a change would
merely eliminate the indeed pretty pointless move from 2 to 3 which
now happens for all domains as their memory gets populated.

> There are no AMD systems which support >48 bit PA space, so 4 levels is
> sufficient for now, but fundamentally details such as the size of GPA
> space should be specified in domain_create() and remain static for the
> lifetime of the domain.

I agree GPA dimensions ought to be static. But the number-of-levels
adjustment the code does has nothing to do with variable GPA
boundaries. Even for a domain with a, say, 36-bit GFN space it
may be beneficial to run with only 3 levels, as long as it has
"little" enough memory assigned. In fact the number of levels the
hardware has to walk is the one aspect you don't even touch in your
reply.

Jan

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

[Xen-devel] [linux-next test] 143701: tolerable FAIL

2019-11-07 Thread osstest service owner
flight 143701 linux-next real [real]
http://logs.test-lab.xenproject.org/osstest/logs/143701/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-arm64-arm64-examine 11 examine-serial/bootloader fail in 143515 pass in 
143701
 test-armhf-armhf-xl-rtds 12 guest-startfail pass in 143515

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt   14 saverestore-support-check fail blocked in 143363
 test-armhf-armhf-libvirt  19 leak-check/check fail in 143515 blocked in 143363
 test-amd64-amd64-xl-rtds  18 guest-localmigrate/x10 fail in 143515 like 143363
 test-armhf-armhf-xl-rtds13 migrate-support-check fail in 143515 never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-check fail in 143515 never pass
 test-amd64-i386-examine   8 reboot   fail  like 143363
 test-amd64-i386-xl-raw7 xen-boot fail  like 143363
 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail  like 143363
 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail  like 143363
 test-amd64-i386-libvirt   7 xen-boot fail  like 143363
 test-amd64-i386-xl-qemuu-debianhvm-amd64  7 xen-boot  fail like 143363
 test-amd64-i386-xl-xsm7 xen-boot fail  like 143363
 test-amd64-i386-xl7 xen-boot fail  like 143363
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail like 
143363
 test-amd64-i386-qemut-rhel6hvm-intel  7 xen-boot  fail like 143363
 test-amd64-i386-xl-shadow 7 xen-boot fail  like 143363
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail like 
143363
 test-amd64-i386-xl-qemut-ws16-amd64  7 xen-boot   fail like 143363
 test-amd64-i386-freebsd10-i386  7 xen-bootfail like 143363
 test-amd64-i386-xl-qemut-win10-i386  7 xen-boot   fail like 143363
 test-amd64-i386-xl-qemut-debianhvm-amd64  7 xen-boot  fail like 143363
 test-amd64-i386-xl-qemuu-win10-i386  7 xen-boot   fail like 143363
 test-amd64-i386-pair 10 xen-boot/src_hostfail  like 143363
 test-amd64-i386-pair 11 xen-boot/dst_hostfail  like 143363
 test-amd64-i386-qemut-rhel6hvm-amd  7 xen-bootfail like 143363
 test-amd64-i386-libvirt-xsm   7 xen-boot fail  like 143363
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow  7 xen-boot   fail like 143363
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  7 xen-boot   fail like 143363
 test-amd64-i386-qemuu-rhel6hvm-amd  7 xen-bootfail like 143363
 test-amd64-i386-qemuu-rhel6hvm-intel  7 xen-boot  fail like 143363
 test-amd64-i386-xl-qemuu-ws16-amd64  7 xen-boot   fail like 143363
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm  7 xen-boot fail like 143363
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm  7 xen-boot   fail like 143363
 test-amd64-i386-xl-qemut-win7-amd64  7 xen-boot   fail like 143363
 test-amd64-i386-xl-pvshim 7 xen-boot fail  like 143363
 test-amd64-i386-xl-qemuu-ovmf-amd64  7 xen-boot   fail like 143363
 test-amd64-i386-xl-qemuu-win7-amd64  7 xen-boot   fail like 143363
 test-amd64-i386-freebsd10-amd64  7 xen-boot   fail like 143363
 test-amd64-amd64-libvirt-vhd 17 guest-start/debian.repeatfail  like 143363
 test-amd64-amd64-xl-qcow219 guest-start/debian.repeatfail  like 143363
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 143363
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 143363
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 143363
 test-armhf-armhf-libvirt-raw 15 guest-start/debian.repeatfail  like 143363
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 143363
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 143363
 test-armhf-armhf-xl-vhd  15 guest-start/debian.repeatfail  like 143363
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 

Re: [Xen-devel] [PATCH 0/3] AMD/IOMMU: re-work mode updating

2019-11-07 Thread Andrew Cooper
On 07/11/2019 07:36, Jan Beulich wrote:
> On 06.11.2019 18:31, Andrew Cooper wrote:
>> On 06/11/2019 15:16, Jan Beulich wrote:
>>> update_paging_mode() in the AMD IOMMU code expects to be invoked with
>>> the PCI devices lock held. The check occurring only when the mode
>>> actually needs updating, the violation of this rule by the majority
>>> of callers did go unnoticed until per-domain IOMMU setup was changed
>>> to do away with on-demand creation of IOMMU page tables.
>>>
>>> Unfortunately the only half way reasonable fix to this that I could
>>> come up with requires more re-work than would seem desirable at this
>>> time of the release process, but addressing the issue seems
>>> unavoidable to me as its manifestation is a regression from the
>>> IOMMU page table setup re-work. The change also isn't without risk
>>> of further regressions - if in patch 2 I've missed a code path that
>>> would also need to invoke the new hook, then this might mean non-
>>> working guests (with passed-through devices on AMD hardware).
>>>
>>> 1: AMD/IOMMU: don't needlessly trigger errors/crashes when unmapping a page
>>> 2: introduce GFN notification for translated domains
>>> 3: AMD/IOMMU: use notify_dfn() hook to update paging mode
>> Having now looked at all three, why don't we just delete the dynamic
>> height of AMD IOMMU pagetables?
>>
>> This series looks suspiciously like it is adding new common
>> infrastructure to work around the fact we're doing something fairly dumb
>> to being with.
>>
>> Hardcoding at 4 levels is, at the very worst, 2 extra pages per domain,
>> and a substantial reduction in the complexity of the IOMMU code.
> Yet an additional level of page walks hardware has to perform. Also
> 4 levels won't cover all possible 52 address bits. And finally, the
> more applicable your "substantial reduction", the less suitable such
> a change may be at this point of the release (but I didn't look at
> this side of things in any detail, so it may well not be an issue).

There is, in practice, no such thing as an HVM guest using 2 levels. 
The VRAM just below the 4G boundary will force a resize to 3 levels
during domain construction, and as a 1-line fix for 4.13, this probably
isn't the worst idea going.

There are no AMD systems which support >48 bit PA space, so 4 levels is
sufficient for now, but fundamentally details such as the size of GPA
space should be specified in domain_create() and remain static for the
lifetime of the domain.

As far as I can tell, it is only AMD systems with IOMMUs which permit
the PA space to be variable, and I still can't help but feeling that
this series is attempting to work around a problem we shouldn't have in
the first place.

~Andrew

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

Re: [Xen-devel] [PATCH 2/3] introduce GFN notification for translated domains

2019-11-07 Thread Jan Beulich
On 07.11.2019 13:10, George Dunlap wrote:
> On 11/7/19 11:47 AM, Jan Beulich wrote:
>> On 07.11.2019 12:35, George Dunlap wrote:
>>> On 11/6/19 3:19 PM, Jan Beulich wrote:
 In order for individual IOMMU drivers (and from an abstract pov also
 architectures) to be able to adjust their data structures ahead of time
 when they might cover only a sub-range of all possible GFNs, introduce
 a notification call used by various code paths potentially installing a
 fresh mapping of a never used GFN (for a particular domain).
>>>
>>> So trying to reverse engineer what's going on here, you mean to say
>>> something like this:
>>>
>>> ---
>>> Individual IOMMU drivers contain adjuct data structures for gfn ranges
>>> contained in the main p2m.  For efficiency, these adjuct data structures
>>> often cover only a subset of the gfn range.  Installing a fresh mapping
>>> of a never-used gfn may require these ranges to be expanded.  Doing this
>>> when the p2m entry is first updated may be problematic because .
>>>
>>> To fix this, implement notify_gfn(), to be called when Xen first becomes
>>> aware that a potentially new gfn may be about to be used.  This will
>>> notify the IOMMU driver about the new gfn, allowing it to expand the
>>> data structures.  It may return -ERESTART (?) for long-running
>>> operations, in which case the operation should be restarted or a
>>> different error if the expansion of the data structure is not possible.
>>>  In the latter case, the entire operation should fail.
>>> ---
>>>
>>> Is that about right?
>>
>> With the exception of the -ERESTART / long running operations aspect,
>> yes. Plus assuming you mean "adjunct" (not "adjuct", which my
>> dictionary doesn't know about).
>>
>>>  Note I've had to make a lot of guesses here about
>>> the functionality and intent.
>>
>> Well, even after seeing your longer description, I don't see what mine
>> doesn't say
> 
> * "Ahead of time" -- ahead of what?

I replaced "time" by "actual mapping requests", realizing that I'm
implying too much here of what is the subject of the next patch.

> * Why do things need to be done ahead of time, rather than at the time
> (for whatever it is)?  (I couldn't even really guess at this, which is
> why I put "".)

This "why" imo really is the subject of the next patch, and hence
gets explained there.

> * To me "notify" doesn't in any way imply that the operation can fail.
> Most modern notifications are FYI only, with no opportunity to prevent
> the thing from happening.  (That's not to say that notify is an
> inappropriate name -- just that by itself it doesn't imply the ability
> to cancel, which seems like a major factor to understanding the intent
> of the patch.)

I'm up for different names; "notify" is what I could think of. It
being able to fail is in line with our more abstract notifier
infrastructure (inherited from Linux) also allowing for NOTIFY_BAD.

 Note that in gnttab_transfer() the notification and lock re-acquire
 handling is best effort only (the guest may not be able to make use of
 the new page in case of failure, but that's in line with the lack of a
 return value check of guest_physmap_add_page() itself).
>>>
>>> Is there a reason we can't just return an error to the caller?
>>
>> Rolling back what has been done by that point would seem rather
>> difficult, which I guess is the reason why the code was written the
>> way it is (prior to my change).
> 
> The phrasing made me think that you were changing it to be best-effort,
> rather than following suit with existing functionality.
> 
> Maybe:
> 
> "Note that before this patch, in gnttab_transfer(), once 
> happens, further errors modifying the physmap are ignored (presumably
> because it would be too complicated to try to roll back at that point).
>  This patch follows suit by ignoring failed notify_gfn()s, simply
> printing out a warning that the gfn may not be accessible due to the
> failure."

Thanks, I'll use this in a slightly extended form.

Jan

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

Re: [Xen-devel] [PATCH 2/3] introduce GFN notification for translated domains

2019-11-07 Thread George Dunlap
On 11/7/19 11:47 AM, Jan Beulich wrote:
> On 07.11.2019 12:35, George Dunlap wrote:
>> On 11/6/19 3:19 PM, Jan Beulich wrote:
>>> In order for individual IOMMU drivers (and from an abstract pov also
>>> architectures) to be able to adjust their data structures ahead of time
>>> when they might cover only a sub-range of all possible GFNs, introduce
>>> a notification call used by various code paths potentially installing a
>>> fresh mapping of a never used GFN (for a particular domain).
>>
>> So trying to reverse engineer what's going on here, you mean to say
>> something like this:
>>
>> ---
>> Individual IOMMU drivers contain adjuct data structures for gfn ranges
>> contained in the main p2m.  For efficiency, these adjuct data structures
>> often cover only a subset of the gfn range.  Installing a fresh mapping
>> of a never-used gfn may require these ranges to be expanded.  Doing this
>> when the p2m entry is first updated may be problematic because .
>>
>> To fix this, implement notify_gfn(), to be called when Xen first becomes
>> aware that a potentially new gfn may be about to be used.  This will
>> notify the IOMMU driver about the new gfn, allowing it to expand the
>> data structures.  It may return -ERESTART (?) for long-running
>> operations, in which case the operation should be restarted or a
>> different error if the expansion of the data structure is not possible.
>>  In the latter case, the entire operation should fail.
>> ---
>>
>> Is that about right?
> 
> With the exception of the -ERESTART / long running operations aspect,
> yes. Plus assuming you mean "adjunct" (not "adjuct", which my
> dictionary doesn't know about).
> 
>>  Note I've had to make a lot of guesses here about
>> the functionality and intent.
> 
> Well, even after seeing your longer description, I don't see what mine
> doesn't say

* "Ahead of time" -- ahead of what?

* Why do things need to be done ahead of time, rather than at the time
(for whatever it is)?  (I couldn't even really guess at this, which is
why I put "".)

* To me "notify" doesn't in any way imply that the operation can fail.
Most modern notifications are FYI only, with no opportunity to prevent
the thing from happening.  (That's not to say that notify is an
inappropriate name -- just that by itself it doesn't imply the ability
to cancel, which seems like a major factor to understanding the intent
of the patch.)

>>> Note that in gnttab_transfer() the notification and lock re-acquire
>>> handling is best effort only (the guest may not be able to make use of
>>> the new page in case of failure, but that's in line with the lack of a
>>> return value check of guest_physmap_add_page() itself).
>>
>> Is there a reason we can't just return an error to the caller?
> 
> Rolling back what has been done by that point would seem rather
> difficult, which I guess is the reason why the code was written the
> way it is (prior to my change).

The phrasing made me think that you were changing it to be best-effort,
rather than following suit with existing functionality.

Maybe:

"Note that before this patch, in gnttab_transfer(), once 
happens, further errors modifying the physmap are ignored (presumably
because it would be too complicated to try to roll back at that point).
 This patch follows suit by ignoring failed notify_gfn()s, simply
printing out a warning that the gfn may not be accessible due to the
failure."

 -George

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

Re: [Xen-devel] [PATCH 1/3] AMD/IOMMU: don't needlessly trigger errors/crashes when unmapping a page

2019-11-07 Thread Paul Durrant
On Wed, 6 Nov 2019 at 15:20, Jan Beulich  wrote:
>
> Unmapping a page which has never been mapped should be a no-op (note how
> it already is in case there was no root page table allocated). There's
> in particular no need to grow the number of page table levels in use,
> and there's also no need to allocate intermediate page tables except
> when needing to split a large page.
>
> Signed-off-by: Jan Beulich 

Reviewed-by: Paul Durrant 

>
> --- a/xen/drivers/passthrough/amd/iommu_map.c
> +++ b/xen/drivers/passthrough/amd/iommu_map.c
> @@ -176,7 +176,7 @@ void iommu_dte_set_guest_cr3(struct amd_
>   * page tables.
>   */
>  static int iommu_pde_from_dfn(struct domain *d, unsigned long dfn,
> -  unsigned long pt_mfn[])
> +  unsigned long pt_mfn[], bool map)
>  {
>  struct amd_iommu_pte *pde, *next_table_vaddr;
>  unsigned long  next_table_mfn;
> @@ -189,6 +189,13 @@ static int iommu_pde_from_dfn(struct dom
>
>  BUG_ON( table == NULL || level < 1 || level > 6 );
>
> +/*
> + * A frame number past what the current page tables can represent can't
> + * possibly have a mapping.
> + */
> +if ( dfn >> (PTE_PER_TABLE_SHIFT * level) )
> +return 0;
> +
>  next_table_mfn = mfn_x(page_to_mfn(table));
>
>  if ( level == 1 )
> @@ -246,6 +253,9 @@ static int iommu_pde_from_dfn(struct dom
>  /* Install lower level page table for non-present entries */
>  else if ( !pde->pr )
>  {
> +if ( !map )
> +return 0;
> +
>  if ( next_table_mfn == 0 )
>  {
>  table = alloc_amd_iommu_pgtable();
> @@ -404,7 +414,7 @@ int amd_iommu_map_page(struct domain *d,
>  }
>  }
>
> -if ( iommu_pde_from_dfn(d, dfn_x(dfn), pt_mfn) || (pt_mfn[1] == 0) )
> +if ( iommu_pde_from_dfn(d, dfn_x(dfn), pt_mfn, true) || (pt_mfn[1] == 0) 
> )
>  {
>  spin_unlock(>arch.mapping_lock);
>  AMD_IOMMU_DEBUG("Invalid IO pagetable entry dfn = %"PRI_dfn"\n",
> @@ -439,24 +449,7 @@ int amd_iommu_unmap_page(struct domain *
>  return 0;
>  }
>
> -/* Since HVM domain is initialized with 2 level IO page table,
> - * we might need a deeper page table for lager dfn now */
> -if ( is_hvm_domain(d) )
> -{
> -int rc = update_paging_mode(d, dfn_x(dfn));
> -
> -if ( rc )
> -{
> -spin_unlock(>arch.mapping_lock);
> -AMD_IOMMU_DEBUG("Update page mode failed dfn = %"PRI_dfn"\n",
> -dfn_x(dfn));
> -if ( rc != -EADDRNOTAVAIL )
> -domain_crash(d);
> -return rc;
> -}
> -}
> -
> -if ( iommu_pde_from_dfn(d, dfn_x(dfn), pt_mfn) || (pt_mfn[1] == 0) )
> +if ( iommu_pde_from_dfn(d, dfn_x(dfn), pt_mfn, false) )
>  {
>  spin_unlock(>arch.mapping_lock);
>  AMD_IOMMU_DEBUG("Invalid IO pagetable entry dfn = %"PRI_dfn"\n",
> @@ -465,8 +458,11 @@ int amd_iommu_unmap_page(struct domain *
>  return -EFAULT;
>  }
>
> -/* mark PTE as 'page not present' */
> -*flush_flags |= clear_iommu_pte_present(pt_mfn[1], dfn_x(dfn));
> +if ( pt_mfn[1] )
> +{
> +/* Mark PTE as 'page not present'. */
> +*flush_flags |= clear_iommu_pte_present(pt_mfn[1], dfn_x(dfn));
> +}
>
>  spin_unlock(>arch.mapping_lock);
>
>
>
> ___
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel

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

Re: [Xen-devel] [PATCH 2/3] introduce GFN notification for translated domains

2019-11-07 Thread Jan Beulich
On 07.11.2019 12:35, George Dunlap wrote:
> On 11/6/19 3:19 PM, Jan Beulich wrote:
>> In order for individual IOMMU drivers (and from an abstract pov also
>> architectures) to be able to adjust their data structures ahead of time
>> when they might cover only a sub-range of all possible GFNs, introduce
>> a notification call used by various code paths potentially installing a
>> fresh mapping of a never used GFN (for a particular domain).
> 
> So trying to reverse engineer what's going on here, you mean to say
> something like this:
> 
> ---
> Individual IOMMU drivers contain adjuct data structures for gfn ranges
> contained in the main p2m.  For efficiency, these adjuct data structures
> often cover only a subset of the gfn range.  Installing a fresh mapping
> of a never-used gfn may require these ranges to be expanded.  Doing this
> when the p2m entry is first updated may be problematic because .
> 
> To fix this, implement notify_gfn(), to be called when Xen first becomes
> aware that a potentially new gfn may be about to be used.  This will
> notify the IOMMU driver about the new gfn, allowing it to expand the
> data structures.  It may return -ERESTART (?) for long-running
> operations, in which case the operation should be restarted or a
> different error if the expansion of the data structure is not possible.
>  In the latter case, the entire operation should fail.
> ---
> 
> Is that about right?

With the exception of the -ERESTART / long running operations aspect,
yes. Plus assuming you mean "adjunct" (not "adjuct", which my
dictionary doesn't know about).

>  Note I've had to make a lot of guesses here about
> the functionality and intent.

Well, even after seeing your longer description, I don't see what mine
doesn't say.

>> Note that in gnttab_transfer() the notification and lock re-acquire
>> handling is best effort only (the guest may not be able to make use of
>> the new page in case of failure, but that's in line with the lack of a
>> return value check of guest_physmap_add_page() itself).
> 
> Is there a reason we can't just return an error to the caller?

Rolling back what has been done by that point would seem rather
difficult, which I guess is the reason why the code was written the
way it is (prior to my change).

Jan

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

Re: [Xen-devel] Ping: [PATCH] build: provide option to disambiguate symbol names

2019-11-07 Thread Jan Beulich
On 07.11.2019 12:03, George Dunlap wrote:
> On 11/7/19 7:20 AM, Jan Beulich wrote:
>> On 24.10.2019 15:31, Jan Beulich wrote:
>>> The .file assembler directives generated by the compiler do not include
>>> any path components (gcc) or just the ones specified on the command line
>>> (clang, at least version 5), and hence multiple identically named source
>>> files (in different directories) may produce identically named static
>>> symbols (in their kallsyms representation). The binary diffing algorithm
>>> used by xen-livepatch, however, depends on having unique symbols.
>>>
>>> Provide a Kconfig option to control the (build) behavior, and if enabled
>>> use objcopy to prepend the (relative to the xen/ subdirectory) path to
>>> the compiler invoked STT_FILE symbols.
> 
> This is a good explanation, and I think the changes make sense.  But
> unfortunately...
> 
>>> Conditionalize explicit .file directive insertion in C files where it
>>> exists just to disambiguate names in a less generic manner; note that
>>> at the same time the redundant emission of STT_FILE symbols gets
>>> suppressed for clang. Assembler files as well as multiply compiled C
>>> ones using __OBJECT_FILE__ are left alone for the time being.
> 
> ...I don't follow this at all.  What does the .file directive do in
> those places, and why is it an issue?

As explained at the beginning of the description, for some dir/file.c
passed to the compiler,
- gcc emits ".file file.c",
- clang emits ".file dir/file.c".
It was a long time ago that we had noticed issues with static symbols
because of gcc omitting the directory part. Hence some .file
directives got inserted in source files where we noticed it would
matter.

As to the "why is it an issue part" - these directives get in the way
of the new mechanism (because we ask for "file.c" symbols to be
renamed, not "dir/file.c" ones).

> And why do we always disable it in clang?

Because, as per above, it's redundant with what the compiler inserts.

Jan

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

Re: [Xen-devel] [PATCH 2/3] introduce GFN notification for translated domains

2019-11-07 Thread George Dunlap
On 11/6/19 3:19 PM, Jan Beulich wrote:
> In order for individual IOMMU drivers (and from an abstract pov also
> architectures) to be able to adjust their data structures ahead of time
> when they might cover only a sub-range of all possible GFNs, introduce
> a notification call used by various code paths potentially installing a
> fresh mapping of a never used GFN (for a particular domain).

So trying to reverse engineer what's going on here, you mean to say
something like this:

---
Individual IOMMU drivers contain adjuct data structures for gfn ranges
contained in the main p2m.  For efficiency, these adjuct data structures
often cover only a subset of the gfn range.  Installing a fresh mapping
of a never-used gfn may require these ranges to be expanded.  Doing this
when the p2m entry is first updated may be problematic because .

To fix this, implement notify_gfn(), to be called when Xen first becomes
aware that a potentially new gfn may be about to be used.  This will
notify the IOMMU driver about the new gfn, allowing it to expand the
data structures.  It may return -ERESTART (?) for long-running
operations, in which case the operation should be restarted or a
different error if the expansion of the data structure is not possible.
 In the latter case, the entire operation should fail.
---

Is that about right?  Note I've had to make a lot of guesses here about
the functionality and intent.

> Note that in gnttab_transfer() the notification and lock re-acquire
> handling is best effort only (the guest may not be able to make use of
> the new page in case of failure, but that's in line with the lack of a
> return value check of guest_physmap_add_page() itself).

Is there a reason we can't just return an error to the caller?

 -George

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

Re: [Xen-devel] [PATCH v5 0/3] x86/boot: Introduce the kernel_info et consortes

2019-11-07 Thread Daniel Kiper
On Wed, Nov 06, 2019 at 09:56:48AM -0800, h...@zytor.com wrote:
> On November 6, 2019 9:03:33 AM PST, Borislav Petkov  wrote:
> >On Mon, Nov 04, 2019 at 04:13:51PM +0100, Daniel Kiper wrote:
> >> Hi,
> >>
> >> Due to very limited space in the setup_header this patch series introduces 
> >> new
> >> kernel_info struct which will be used to convey information from the 
> >> kernel to
> >> the bootloader. This way the boot protocol can be extended regardless of 
> >> the
> >> setup_header limitations. Additionally, the patch series introduces some
> >> convenience features like the setup_indirect struct and the
> >> kernel_info.setup_type_max field.
> >
> >That's all fine and dandy but I'm missing an example about what that'll
> >be used for, in practice.
> >
> >Thx.
>
> For one thing, we already have people asking for more than 4 GiB worth
> of initramfs, and especially with initramfs that huge it would make a
> *lot* of sense to allow loading it in chunks without having to
> concatenate them. I have been asking for a long time for initramfs
> creators to split the kernel-dependent and kernel independent parts
> into separate initramfs modules.

Another user of this patchset is the TrenchBoot project on which we are
working on. We have to introduce separate entry point for Intel TXT MLE
startup code. That is why we need the kernel_info struct.

Daniel

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

[Xen-devel] [PATCH v2 0/2] xen/gntdev: sanitize user interface handling

2019-11-07 Thread Juergen Gross
The Xen gntdev driver's checking of the number of allowed mapped pages
is in need of some sanitizing work.

Changes in V2:
- enhanced commit message of patch 1 (Andrew Cooper)

Juergen Gross (2):
  xen/gntdev: replace global limit of mapped pages by limit per call
  xen/gntdev: switch from kcalloc() to kvcalloc()

 drivers/xen/gntdev-common.h |  2 +-
 drivers/xen/gntdev-dmabuf.c | 11 +++--
 drivers/xen/gntdev.c| 55 +++--
 3 files changed, 27 insertions(+), 41 deletions(-)

-- 
2.16.4


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

[Xen-devel] [PATCH v2 1/2] xen/gntdev: replace global limit of mapped pages by limit per call

2019-11-07 Thread Juergen Gross
Today there is a global limit of pages mapped via /dev/xen/gntdev set
to 1 million pages per default. There is no reason why that limit is
existing, as total number of grant mappings is limited by the
hypervisor anyway and preferring kernel mappings over userspace ones
doesn't make sense. It should be noted that the gntdev device is
usable by root only.

Additionally checking of that limit is fragile, as the number of pages
to map via one call is specified in a 32-bit unsigned variable which
isn't tested to stay within reasonable limits (the only test is the
value to be <= zero, which basically excludes only calls without any
mapping requested). So trying to map e.g. 0x pages while
already nearly 100 pages are mapped will effectively lower the
global number of mapped pages such that a parallel call mapping a
reasonable amount of pages can succeed in spite of the global limit
being violated.

So drop the global limit and introduce per call limit instead. This
per call limit (default: 65536 grant mappings) protects against
allocating insane large arrays in the kernel for doing a hypercall
which will fail anyway in case a user is e.g. trying to map billions
of pages.

Signed-off-by: Juergen Gross 
Reviewed-by: Oleksandr Andrushchenko 
---
 drivers/xen/gntdev-common.h |  2 +-
 drivers/xen/gntdev-dmabuf.c | 11 +++
 drivers/xen/gntdev.c| 24 +++-
 3 files changed, 11 insertions(+), 26 deletions(-)

diff --git a/drivers/xen/gntdev-common.h b/drivers/xen/gntdev-common.h
index 2f8b949c3eeb..0e5d4660e7b8 100644
--- a/drivers/xen/gntdev-common.h
+++ b/drivers/xen/gntdev-common.h
@@ -87,7 +87,7 @@ void gntdev_add_map(struct gntdev_priv *priv, struct 
gntdev_grant_map *add);
 
 void gntdev_put_map(struct gntdev_priv *priv, struct gntdev_grant_map *map);
 
-bool gntdev_account_mapped_pages(int count);
+bool gntdev_test_page_count(unsigned int count);
 
 int gntdev_map_grant_pages(struct gntdev_grant_map *map);
 
diff --git a/drivers/xen/gntdev-dmabuf.c b/drivers/xen/gntdev-dmabuf.c
index 2c4f324f8626..63f0857bf62d 100644
--- a/drivers/xen/gntdev-dmabuf.c
+++ b/drivers/xen/gntdev-dmabuf.c
@@ -446,7 +446,7 @@ dmabuf_exp_alloc_backing_storage(struct gntdev_priv *priv, 
int dmabuf_flags,
 {
struct gntdev_grant_map *map;
 
-   if (unlikely(count <= 0))
+   if (unlikely(gntdev_test_page_count(count)))
return ERR_PTR(-EINVAL);
 
if ((dmabuf_flags & GNTDEV_DMA_FLAG_WC) &&
@@ -459,11 +459,6 @@ dmabuf_exp_alloc_backing_storage(struct gntdev_priv *priv, 
int dmabuf_flags,
if (!map)
return ERR_PTR(-ENOMEM);
 
-   if (unlikely(gntdev_account_mapped_pages(count))) {
-   pr_debug("can't map %d pages: over limit\n", count);
-   gntdev_put_map(NULL, map);
-   return ERR_PTR(-ENOMEM);
-   }
return map;
 }
 
@@ -771,7 +766,7 @@ long gntdev_ioctl_dmabuf_exp_from_refs(struct gntdev_priv 
*priv, int use_ptemod,
if (copy_from_user(, u, sizeof(op)) != 0)
return -EFAULT;
 
-   if (unlikely(op.count <= 0))
+   if (unlikely(gntdev_test_page_count(op.count)))
return -EINVAL;
 
refs = kcalloc(op.count, sizeof(*refs), GFP_KERNEL);
@@ -818,7 +813,7 @@ long gntdev_ioctl_dmabuf_imp_to_refs(struct gntdev_priv 
*priv,
if (copy_from_user(, u, sizeof(op)) != 0)
return -EFAULT;
 
-   if (unlikely(op.count <= 0))
+   if (unlikely(gntdev_test_page_count(op.count)))
return -EINVAL;
 
gntdev_dmabuf = dmabuf_imp_to_refs(priv->dmabuf_priv,
diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 81401f386c9c..0578d369e537 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -55,12 +55,10 @@ MODULE_AUTHOR("Derek G. Murray , 
"
  "Gerd Hoffmann ");
 MODULE_DESCRIPTION("User-space granted page access driver");
 
-static int limit = 1024*1024;
-module_param(limit, int, 0644);
-MODULE_PARM_DESC(limit, "Maximum number of grants that may be mapped by "
-   "the gntdev device");
-
-static atomic_t pages_mapped = ATOMIC_INIT(0);
+static unsigned int limit = 64*1024;
+module_param(limit, uint, 0644);
+MODULE_PARM_DESC(limit,
+   "Maximum number of grants that may be mapped by one mapping request");
 
 static int use_ptemod;
 #define populate_freeable_maps use_ptemod
@@ -72,9 +70,9 @@ static struct miscdevice gntdev_miscdev;
 
 /* -- */
 
-bool gntdev_account_mapped_pages(int count)
+bool gntdev_test_page_count(unsigned int count)
 {
-   return atomic_add_return(count, _mapped) > limit;
+   return !count || count > limit;
 }
 
 static void gntdev_print_maps(struct gntdev_priv *priv,
@@ -242,8 +240,6 @@ void gntdev_put_map(struct gntdev_priv *priv, struct 
gntdev_grant_map *map)
if (!refcount_dec_and_test(>users))
return;
 
-   atomic_sub(map->count, _mapped);
-

[Xen-devel] [PATCH v2 2/2] xen/gntdev: switch from kcalloc() to kvcalloc()

2019-11-07 Thread Juergen Gross
With sufficient many pages to map gntdev can reach order 9 allocation
sizes. As there is no need to have physically contiguous buffers switch
to kvcalloc() in order to avoid failing allocations.

Signed-off-by: Juergen Gross 
Reviewed-by: Oleksandr Andrushchenko 
---
 drivers/xen/gntdev.c | 31 ---
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 0578d369e537..f1bc0d269e12 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -113,14 +113,14 @@ static void gntdev_free_map(struct gntdev_grant_map *map)
gnttab_free_pages(map->count, map->pages);
 
 #ifdef CONFIG_XEN_GRANT_DMA_ALLOC
-   kfree(map->frames);
+   kvfree(map->frames);
 #endif
-   kfree(map->pages);
-   kfree(map->grants);
-   kfree(map->map_ops);
-   kfree(map->unmap_ops);
-   kfree(map->kmap_ops);
-   kfree(map->kunmap_ops);
+   kvfree(map->pages);
+   kvfree(map->grants);
+   kvfree(map->map_ops);
+   kvfree(map->unmap_ops);
+   kvfree(map->kmap_ops);
+   kvfree(map->kunmap_ops);
kfree(map);
 }
 
@@ -134,12 +134,13 @@ struct gntdev_grant_map *gntdev_alloc_map(struct 
gntdev_priv *priv, int count,
if (NULL == add)
return NULL;
 
-   add->grants= kcalloc(count, sizeof(add->grants[0]), GFP_KERNEL);
-   add->map_ops   = kcalloc(count, sizeof(add->map_ops[0]), GFP_KERNEL);
-   add->unmap_ops = kcalloc(count, sizeof(add->unmap_ops[0]), GFP_KERNEL);
-   add->kmap_ops  = kcalloc(count, sizeof(add->kmap_ops[0]), GFP_KERNEL);
-   add->kunmap_ops = kcalloc(count, sizeof(add->kunmap_ops[0]), 
GFP_KERNEL);
-   add->pages = kcalloc(count, sizeof(add->pages[0]), GFP_KERNEL);
+   add->grants= kvcalloc(count, sizeof(add->grants[0]), GFP_KERNEL);
+   add->map_ops   = kvcalloc(count, sizeof(add->map_ops[0]), GFP_KERNEL);
+   add->unmap_ops = kvcalloc(count, sizeof(add->unmap_ops[0]), GFP_KERNEL);
+   add->kmap_ops  = kvcalloc(count, sizeof(add->kmap_ops[0]), GFP_KERNEL);
+   add->kunmap_ops = kvcalloc(count,
+  sizeof(add->kunmap_ops[0]), GFP_KERNEL);
+   add->pages = kvcalloc(count, sizeof(add->pages[0]), GFP_KERNEL);
if (NULL == add->grants||
NULL == add->map_ops   ||
NULL == add->unmap_ops ||
@@ -158,8 +159,8 @@ struct gntdev_grant_map *gntdev_alloc_map(struct 
gntdev_priv *priv, int count,
if (dma_flags & (GNTDEV_DMA_FLAG_WC | GNTDEV_DMA_FLAG_COHERENT)) {
struct gnttab_dma_alloc_args args;
 
-   add->frames = kcalloc(count, sizeof(add->frames[0]),
- GFP_KERNEL);
+   add->frames = kvcalloc(count, sizeof(add->frames[0]),
+  GFP_KERNEL);
if (!add->frames)
goto err;
 
-- 
2.16.4


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

Re: [Xen-devel] Ping: [PATCH] build: provide option to disambiguate symbol names

2019-11-07 Thread George Dunlap
On 11/7/19 7:20 AM, Jan Beulich wrote:
> On 24.10.2019 15:31, Jan Beulich wrote:
>> The .file assembler directives generated by the compiler do not include
>> any path components (gcc) or just the ones specified on the command line
>> (clang, at least version 5), and hence multiple identically named source
>> files (in different directories) may produce identically named static
>> symbols (in their kallsyms representation). The binary diffing algorithm
>> used by xen-livepatch, however, depends on having unique symbols.
>>
>> Provide a Kconfig option to control the (build) behavior, and if enabled
>> use objcopy to prepend the (relative to the xen/ subdirectory) path to
>> the compiler invoked STT_FILE symbols.

This is a good explanation, and I think the changes make sense.  But
unfortunately...

>> Conditionalize explicit .file directive insertion in C files where it
>> exists just to disambiguate names in a less generic manner; note that
>> at the same time the redundant emission of STT_FILE symbols gets
>> suppressed for clang. Assembler files as well as multiply compiled C
>> ones using __OBJECT_FILE__ are left alone for the time being.

...I don't follow this at all.  What does the .file directive do in
those places, and why is it an issue?  And why do we always disable it
in clang?

 -George


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

Re: [Xen-devel] Latest development (master) Xen fails to boot on HP ProLiant DL20 GEN10

2019-11-07 Thread Jan Beulich
On 01.10.2019 00:38, Roman Shaposhnik wrote:
> Btw, forgot to attach the patch with maxcpus=2 -- interestingly enough
> Xen seems to hang much further down than before (basically after
> attempting to build out Dom0)

Have you meanwhile had a chance to retry with latest microcode?

Jan

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

[Xen-devel] AMD Rome booting issues with the latest Xen 4.13

2019-11-07 Thread Sergey Dyasli
Hello,

The latest upstream fails to boot on AMD Rome with this:

(XEN) [   10.082154] ENABLING IO-APIC IRQs
(XEN) [   10.087789]  -> Using new ACK method
(XEN) [   10.093738] Assertion 'get_rte_index(rte) == offset' failed at 
iommu_intr.c:328

Full log is available at https://paste.debian.net/1114963/

--
Thanks,
Sergey

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

Re: [Xen-devel] [PATCH 1/3] AMD/IOMMU: don't needlessly trigger errors/crashes when unmapping a page

2019-11-07 Thread Jan Beulich
On 06.11.2019 18:12, Andrew Cooper wrote:
> On 06/11/2019 15:18, Jan Beulich wrote:
>> Unmapping a page which has never been mapped should be a no-op (note how
>> it already is in case there was no root page table allocated).
> 
> Which function are you talking about here?  iommu_pde_from_dfn() will
> BUG() if no root was set up.

amd_iommu_unmap_page() has such a check first thing.

>> There's
>> in particular no need to grow the number of page table levels in use,
>> and there's also no need to allocate intermediate page tables except
>> when needing to split a large page.
> 
> To be honest, I've never been convinced that dynamically changing the
> number of levels in the AMD IOMMU tables is clever.  It should be fixed
> at 4 (like everything else) and suddenly a lot of runtime complexity
> disappears.  (I'm fairly confident that we'll need a domain create
> parameter to support 5 level paging in a rational way, so we won't even
> include walk-length gymnastics then either.)

5-level paging for the CPU 1st-stage-translation is imo pretty orthogonal
to needing 5 levels of paging for 2nd-stage-translation (which also is
what the IOMMU code here is about).

>> Signed-off-by: Jan Beulich 
> 
> Acked-by: Andrew Cooper 

Thanks, Jan

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

Re: [Xen-devel] [PATCH v2 08/15] xen/gntdev: Use select for DMA_SHARED_BUFFER

2019-11-07 Thread Jürgen Groß

On 28.10.19 21:10, Jason Gunthorpe wrote:

From: Jason Gunthorpe 

DMA_SHARED_BUFFER can not be enabled by the user (it represents a library
set in the kernel). The kconfig convention is to use select for such
symbols so they are turned on implicitly when the user enables a kconfig
that needs them.

Otherwise the XEN_GNTDEV_DMABUF kconfig is overly difficult to enable.

Fixes: 932d6562179e ("xen/gntdev: Add initial support for dma-buf UAPI")
Cc: Oleksandr Andrushchenko 
Cc: Boris Ostrovsky 
Cc: xen-devel@lists.xenproject.org
Cc: Juergen Gross 
Cc: Stefano Stabellini 
Reviewed-by: Juergen Gross 
Reviewed-by: Oleksandr Andrushchenko 
Signed-off-by: Jason Gunthorpe 


Applied to xen/tip.git for-linus-5.5a


Juergen

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

Re: [Xen-devel] [PATCH 1/1] kdd.c: Add support for initial handshake in KD protocol for Win 7, 8 and 10 (64 bit)

2019-11-07 Thread Paul Durrant
On Wed, 6 Nov 2019 at 02:24, Julian Tuminaro  wrote:
>
> From: "julian.tumin...@gmail.com" 
>
>
> Current implementation of find_os is based on the hard-coded values for
> different Windows version. It uses the value for get the address to
> start looking for DOS header in the given specified range. However, this
> is not scalable to all version of Windows as it will require us to keep
> adding new entries and also due to KASLR, chances of not hitting the PE
> header is significant. We implement a way for 64-bit systems to use IDT
> entry to get a valid exception/interrupt handler and then move back into
> the memory to find the valid DOS header. Since IDT entries are protected
> by PatchGuard, we think our assumption that IDT entries will not be
> corrupted is valid for our purpose. Once we have the image base, we
> search for the DBGKD_GET_VERSION64 structure type in .data section to
> get information required for handshake.
>
> Currently, this is a work in progress feature and current patch only
> supports the handshake and memory read/write on 64-bit systems.
>
> Signed-off-by: Jenish Rakholiya 
> Signed-off-by: Julian Tuminaro 

Julian,

Thanks for the patch. This work is great progress for kdd. Comments
below... Mainly style-related but I think there's also some clean-up
needed and a few tweaks.

> ---
>  tools/debugger/kdd/kdd.c | 389 ++-
>  1 file changed, 388 insertions(+), 1 deletion(-)
>
> diff --git a/tools/debugger/kdd/kdd.c b/tools/debugger/kdd/kdd.c
> index fb8c645355..407b70a21c 100644
> --- a/tools/debugger/kdd/kdd.c
> +++ b/tools/debugger/kdd/kdd.c
> @@ -51,6 +51,8 @@
>
>  #include "kdd.h"
>
> +// TODO: make fix this to actually use address instead of offset?
> +// TODO: Maybe clean something as well?

These TODOs are a not really well enough described to be committed
IMO. I'm totally fine with having TODO or FIXME comments, but they
ought to be clear enough for someone else picking up the code.

>  /* Windows version details */
>  typedef struct {
>  uint32_t build;
> @@ -62,6 +64,7 @@ typedef struct {
>  uint32_t version;   /* +-> NtBuildNumber */
>  uint32_t modules;   /* +-> PsLoadedModuleList */
>  uint32_t prcbs; /* +-> KiProcessorBlock */
> +uint32_t kddl;

Comment for this field perhaps?

>  } kdd_os;
>
>  /* State of the debugger stub */
> @@ -85,6 +88,106 @@ typedef struct {
>  kdd_os os; /* OS-specific magic numbers 
> */
>  } kdd_state;
>
> +/**
> + * @brief Size of pointer on 64 machine
> + */
> +#define SIZE_PTR64 8
> +
> +/**
> + * @brief Size of pointer on 32 machine
> + */
> +#define SIZE_PTR32 4
> +
> +
> +/*
> + * PE and DOS Header related offsets
> + */
> +
> +/**
> + * @brief Offset in DOS header to look for PE header
> + */
> +#define DOS_HDR_PE_OFF 0x3c
> +
> +/**
> + * @brief Size of PE header offset field in DOS header
> + */
> +#define DOS_HDR_PE_SZ 4
> +
> +/**
> + * @brief Offset of number of sections field in PE header
> + */
> +#define PE_NUM_SECTION_OFF 0x6
> +
> +/**
> + * @brief Size of number of sections field in PE header
> + */
> +#define PE_NUM_SECTION_SZ 2
> +
> +/**
> + * @brief Offset of optional header size field in PE header
> + */
> +#define PE_OPT_HDR_SZ_OFF 0x14
> +
> +/**
> + * @brief Size of optional header size field in PE header
> + */
> +#define PE_OPT_HDR_SZ_SZ 2
> +
> +/**
> + * @brief Size of PE header
> + */
> +#define PE_HDR_SZ 0x18
> +
> +/**
> + * @brief Size of section header entries in PE
> + */
> +#define PE_SECT_ENT_SZ 0x28
> +
> +/**
> + * @brief Offset of name field in section header entry
> + */
> +#define PE_SECT_NAME_OFF 0
> +
> +/**
> + * @brief Size of name field in section header entry
> + */
> +#define PE_SECT_NAME_SZ 0x8
> +
> +/**
> + * @brief Offset of virtual address (RVA) field in section header entry
> + */
> +#define PE_SECT_RVA_OFF 0xc
> +
> +/**
> + * @brief Offset of virtual size field in section header entry
> + */
> +#define PE_SECT_VSIZE_OFF 0x8
> +
> +/**
> + * @brief Size of DBGKD_GET_VERSION64 struct
> + */
> +#define DBGKD_GET_VERSION64_SZ 0x28
> +
> +/**
> + * @brief Offset of KERN_BASE in DBGKD_GET_VERSION64 struct
> + */
> +#define DBGKD_KERN_BASE_OFF 0x10
> +
> +/**
> + * @brief Offset of PsLoadedModulesList in DBGKD_GET_VERSION64 struct
> + */
> +#define DBGKD_MOD_LIST_OFF 0x18
> +
> +/**
> + * @brief Offset of DebuggerDataList in DBGKD_GET_VERSION64 struct
> + */
> +#define DBGKD_KDDL_OFF 0x20
> +
> +/**
> + * @brief Offset of DebuggerDataList in DBGKD_GET_VERSION64 struct
> + */
> +#define DBGKD_MINOR_OFF 0x2
> +
>  
> /*
>   *  Utility functions
>   */
> @@ -390,6 +493,284 @@ static void find_os(kdd_state *s)
>  s->os = unknown_os;
>  }
>
> +#if 0
> +/**
> + * @brief Temporary function for printing memory dump while