Re: [PATCH 10/12] xen/arm: if is_domain_direct_mapped use native UART address for vPL011

[Stefano Stabellini]

On Fri, 1 May 2020, Julien Grall wrote:
> On 01/05/2020 02:26, Stefano Stabellini wrote:
> > On Wed, 15 Apr 2020, Julien Grall wrote:
> > > Hi Stefano,
> > > 
> > > On 15/04/2020 02:02, Stefano Stabellini wrote:
> > > > We always use a fix address to map the vPL011 to domains. The address
> > > > could be a problem for domains that are directly mapped.
> > > > 
> > > > Instead, for domains that are directly mapped, reuse the address of the
> > > > physical UART on the platform to avoid potential clashes.
> > > 
> > > How do you know the physical UART MMIO region is big enough to fit the
> > > PL011?
> > 
> > That cannot be because the vPL011 MMIO size is 1 page, which is the
> > minimum right?
> 
> No, there are platforms out with multiple UARTs in the same page (see sunxi
> for instance).

But if there are multiple UARTs sharing the same page, and the first one
is used by Xen, there is no way to assign one of the secondary UARTs to
a domU. So there would be no problem choosing the physical UART address
for the virtual PL011.
 
 
> > > What if the user want to assign the physical UART to the domain + the
> > > vpl011?
> > 
> > A user can assign a UART to the domain, but it cannot assign the UART
> > used by Xen (DTUART), which is the one we are using here to get the
> > physical information.
> > 
> > 
> > (If there is no UART used by Xen then we'll fall back to the virtual
> > addresses. If they conflict we return error and let the user fix the
> > configuration.)
> 
> If there is no UART in Xen, how the user will know the addresses conflicted?
> Earlyprintk?

That's a good question. Yes, I think earlyprintk would be the only way.

Re: [PATCH 09/12] xen/arm: if is_domain_direct_mapped use native addresses for GICv3

[Stefano Stabellini]

On Fri, 1 May 2020, Julien Grall wrote:
> Hi Stefano,
> 
> On 01/05/2020 02:31, Stefano Stabellini wrote:
> > On Wed, 15 Apr 2020, Julien Grall wrote:
> > > > diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> > > > index 4e60ba15cc..4cf430f865 100644
> > > > --- a/xen/arch/arm/vgic-v3.c
> > > > +++ b/xen/arch/arm/vgic-v3.c
> > > > @@ -1677,13 +1677,25 @@ static int vgic_v3_domain_init(struct domain *d)
> > > 
> > > 
> > > I think you also want to modify vgic_v3_max_rdist_count().
> > 
> > I don't think so: domUs even direct-mapped still only get 1 rdist
> > region. This patch is not changing the layout of the domU gic, it is
> > only finding a "hole" in the physical address space to make sure there
> > are no conflicts (or at least minimize the chance of conflicts.)
> 
> How do you know the "hole" is big enough?
> 
> > 
> > > > * Domain 0 gets the hardware address.
> > > > * Guests get the virtual platform layout.
> > > 
> > > This comment needs to be updated.
> > 
> > Yep, I'll do
> > 
> > 
> > > > */
> > > > -if ( is_hardware_domain(d) )
> > > > +if ( is_domain_direct_mapped(d) )
> > > >{
> > > >unsigned int first_cpu = 0;
> > > > +unsigned int nr_rdist_regions;
> > > >  d->arch.vgic.dbase = vgic_v3_hw.dbase;
> > > >-for ( i = 0; i < vgic_v3_hw.nr_rdist_regions; i++ )
> > > > +if ( is_hardware_domain(d) )
> > > > +{
> > > > +nr_rdist_regions = vgic_v3_hw.nr_rdist_regions;
> > > > +d->arch.vgic.intid_bits = vgic_v3_hw.intid_bits;
> > > > +}
> > > > +else
> > > > +{
> > > > +nr_rdist_regions = 1;
> > > 
> > > What does promise your the rdist region will be big enough to cater all
> > > the
> > > re-distributors for your domain?
> > 
> > Good point. I'll add an explicit check for that with at least a warning.
> > I don't think we want to return error because the configuration it is
> > still likely to work.
> 
> No it is not going to work. Imagine you have have a guest with 3 vCPUs but the
> first re-distributor region can only cater 2 re-distributor. How is this going
> to be fine to continue?
> 
> For dom0, we are re-using the same hole but possibly not all of them. Why
> can't we do that for domU?

I implemented what you suggested

Re: [PATCH 08/12] xen/arm: if is_domain_direct_mapped use native addresses for GICv2

[Stefano Stabellini]

On Fri, 1 May 2020, Julien Grall wrote:
> On 01/05/2020 02:26, Stefano Stabellini wrote:
> > On Wed, 15 Apr 2020, Julien Grall wrote:
> > > On 15/04/2020 02:02, Stefano Stabellini wrote:
> > > > Today we use native addresses to map the GICv2 for Dom0 and fixed
> > > > addresses for DomUs.
> > > > 
> > > > This patch changes the behavior so that native addresses are used for
> > > > any domain that is_domain_direct_mapped.
> > > > 
> > > > Signed-off-by: Stefano Stabellini 
> > > > ---
> > > >xen/arch/arm/domain_build.c | 10 +++---
> > > >xen/arch/arm/vgic-v2.c  | 12 ++--
> > > >xen/arch/arm/vgic/vgic-v2.c |  2 +-
> > > >xen/include/asm-arm/vgic.h  |  1 +
> > > >4 files changed, 15 insertions(+), 10 deletions(-)
> > > > 
> > > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > > > index 627e0c5e8e..303bee60f6 100644
> > > > --- a/xen/arch/arm/domain_build.c
> > > > +++ b/xen/arch/arm/domain_build.c
> > > > @@ -1643,8 +1643,12 @@ static int __init make_gicv2_domU_node(struct
> > > > kernel_info *kinfo)
> > > >int res = 0;
> > > >__be32 reg[(GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) *
> > > > 2];
> > > >__be32 *cells;
> > > > +struct domain *d = kinfo->d;
> > > > +char buf[38];
> > > >-res = fdt_begin_node(fdt,
> > > > "interrupt-controller@"__stringify(GUEST_GICD_BASE));
> > > > +snprintf(buf, sizeof(buf), "interrupt-controller@%"PRIx64,
> > > > + d->arch.vgic.dbase);
> > > > +res = fdt_begin_node(fdt, buf);
> > > >if ( res )
> > > >return res;
> > > >@@ -1666,9 +1670,9 @@ static int __init make_gicv2_domU_node(struct
> > > > kernel_info *kinfo)
> > > >  cells = [0];
> > > >dt_child_set_range(, GUEST_ROOT_ADDRESS_CELLS,
> > > > GUEST_ROOT_SIZE_CELLS,
> > > > -   GUEST_GICD_BASE, GUEST_GICD_SIZE);
> > > > +   d->arch.vgic.dbase, GUEST_GICD_SIZE);
> > > >dt_child_set_range(, GUEST_ROOT_ADDRESS_CELLS,
> > > > GUEST_ROOT_SIZE_CELLS,
> > > > -   GUEST_GICC_BASE, GUEST_GICC_SIZE);
> > > > +   d->arch.vgic.cbase, GUEST_GICC_SIZE);
> > > 
> > > You can't use the native base address and not the native size.
> > > Particularly,
> > > this is going to screw any GIC using 8KB alias.
> > 
> > I don't follow why it could cause problems with a GIC using the 8KB
> > alias. Maybe an example (even a fake example) would help.
> 
> The GICC interface is composed of the two 4KB pages. In some of the
> implementation, each pages starts at a 64KB aligned address. Each page are
> also aliased every 4KB in the 64KB region.
> 
> For guest, we don't expose the full 128KB region but only part of it (8KB). So
> the guest interface is the same regardless the underlying implementation of
> the GIC.
> 
> vgic.cbase points to the beginning of the first region. So what you are
> mapping is the first 8KB of the first region. The second region is not mapped
> at all.
> 
> As the pages are aliased, the trick we use is to map from vgic.cbase + 60KB
> (vgic_v2.hw.aliased_offset). This means the 2 pages will now be contiguous in
> the guest physical memory.

I understood the issue and I fixed it by applying
vgic_v2.hw.aliased_offset to vbase and cbase. (Although only vbase is
actually necessary as far as I can tell.)

Re: [PATCH 03/12] xen/arm: introduce 1:1 mapping for domUs

[Stefano Stabellini]

On Fri, 1 May 2020, Julien Grall wrote:
> On 01/05/2020 02:26, Stefano Stabellini wrote:
> > On Wed, 15 Apr 2020, Julien Grall wrote:
> > > On 15/04/2020 02:02, Stefano Stabellini wrote:
> > > > In some cases it is desirable to map domU memory 1:1 (guest physical ==
> > > > physical.) For instance, because we want to assign a device to the domU
> > > > but the IOMMU is not present or cannot be used. In these cases, other
> > > > mechanisms should be used for DMA protection, e.g. a MPU.
> > > 
> > > I am not against this, however the documentation should clearly explain
> > > that
> > > you are making your platform insecure unless you have other mean for DMA
> > > protection.
> > 
> > I'll expand the docs
> > 
> > 
> > > > 
> > > > This patch introduces a new device tree option for dom0less guests to
> > > > request a domain to be directly mapped. It also specifies the memory
> > > > ranges. This patch documents the new attribute and parses it at boot
> > > > time. (However, the implementation of 1:1 mapping is missing and just
> > > > BUG() out at the moment.)  Finally the patch sets the new direct_map
> > > > flag for DomU domains.
> > > > 
> > > > Signed-off-by: Stefano Stabellini 
> > > > ---
> > > >docs/misc/arm/device-tree/booting.txt | 13 +++
> > > >docs/misc/arm/passthrough-noiommu.txt | 35 ++
> > > >xen/arch/arm/domain_build.c   | 52
> > > > +--
> > > >3 files changed, 98 insertions(+), 2 deletions(-)
> > > >create mode 100644 docs/misc/arm/passthrough-noiommu.txt
> > > > 
> > > > diff --git a/docs/misc/arm/device-tree/booting.txt
> > > > b/docs/misc/arm/device-tree/booting.txt
> > > > index 5243bc7fd3..fce5f7ed5a 100644
> > > > --- a/docs/misc/arm/device-tree/booting.txt
> > > > +++ b/docs/misc/arm/device-tree/booting.txt
> > > > @@ -159,6 +159,19 @@ with the following properties:
> > > >used, or GUEST_VPL011_SPI+1 if vpl011 is enabled, whichever is
> > > >greater.
> > > >+- direct-map
> > > > +
> > > > +Optional. An array of integer pairs specifying addresses and sizes.
> > > > +direct_map requests the memory of the domain to be 1:1 mapped with
> > > > +the memory ranges specified as argument. Only sizes that are a
> > > > +power of two number of pages are allowed.
> > > > +
> > > > +- #direct-map-addr-cells and #direct-map-size-cells
> > > > +
> > > > +The number of cells to use for the addresses and for the sizes in
> > > > +direct-map. Default and maximum are 2 cells for both addresses and
> > > > +sizes.
> > > > +
> > > 
> > > As this is going to be mostly used for passthrough, can't we take
> > > advantage of
> > > the partial device-tree and describe the memory region using memory node?
> > 
> > With the system device tree bindings that are under discussion the role
> > of the partial device tree might be reduce going forward, and might even
> > go away in the long term. For this reason, I would prefer not to add
> > more things to the partial device tree.
> 
> Was the interface you suggested approved by the committee behind system device
> tree? If not, we will still have to support your proposal + whatever the
> committee come up with. So I am not entirely sure why using the partial
> device-tree will be an issue.

Not yet


> It is actually better to keep everything in the partial device-tree as it
> would avoid to clash with whatever you come up with the system device tree.

I don't think we want to support both in the long term. The closer we
are to it the better for transitioning.


> Also, I don't think the partial device-tree could ever go away at least in
> Xen. This is an external interface we provide to the user, removing it would
> mean users would not be able to upgrade from Xen 4.x to 4.y without any major
> rewrite of there DT.

I don't want to put the memory ranges inside the multiboot,device-tree
module because that is clearly for device assignment:

"Device Assignment (Passthrough) is supported by adding another module,
alongside the kernel and ramdisk, with the device tree fragment
corresponding to the device node to assign to the guest."

One could do 1:1 memory mapping without device assignment.


Genuine question: did we write down any compatibility promise on that
interface? If so, do you know where? I'd like to take a look.

In any case backward compatible interfaces can be deprecated although it
takes time. Alternatively it could be made optional (even for device
assignment). So expanding its scope beyond device assignment
configuration it is not a good idea.

Re: [RFC PATCH] docs/designs: domB design document

[Christopher Clark]

On Thu, May 7, 2020 at 1:15 AM Jan Beulich  wrote:
>
> On 06.05.2020 05:23, Christopher Clark wrote:
> > +It is with this understanding as presented that the DomB project used as 
> > the
> > +basis for the development of its multiple domain boot capability for Xen. 
> > Within
> > +the remainder of this document is a detailed explanation of the multiple 
> > domain
> > +boot, the objectives it strives to achieve, the structure behind the 
> > approach,
> > +the sequence events in a boot, a contrasting with ARM’s dom0less, and 
> > closing
> > +with some exemplar implementations.
>
> May I ask that along with dom0less you also explain the (non-)relationship
> to the late-Dom0 model we've been having for a number of years? Some
> aspects of what the boot domain does look, to me, quite similar.

I haven't seen the term 'late-Dom0' used before, so am inferring that
you might mean the 'late hardware domain' feature of Xen, in which
case: yes, thanks - we will add a contrast section on how DomB relates
to that. At this (early) point, I think that we should be able to
retire/replace Xen's late hardware domain implementation in favour of
DomB, if that direction is acceptable to the community; so we will
look into how it relates.

> Apart from this one immediate detail I'm curious about (and that I also
> don't know/recall how it gets handled with dom0less): Death of Dom0 in a
> traditional setup is a signal to Xen to reboot the host. With any of the
> boot time created domains not functioning anymore, the intended purpose
> of the host may no longer be fulfilled. But of course there may be a
> subset of "optional" domains. As a result - are there any intentions
> towards identifying under what conditions it may be better to reboot the
> host than wait for human interaction?

Excellent point; we have given it some limited consideration -- eg.
something should be able to be expressed in the per-domain metadata
supplied in the Launch Control Module, to set state that is held in
Xen's domain struct for acting upon on domain exit -- but it is not
included in the design document yet and indeed it ought to be.

Thanks for the review.

Christopher

[linux-linus test] 150081: tolerable FAIL - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10   fail  like 150064
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 150064
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 150064
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 150064
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 150064
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 150064
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 150064
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 150064
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 150064
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-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 13 migrate-support-checkfail   never pass
 test-amd64-i386-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-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-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-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  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-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemut-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-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass

version targeted for testing:
 linux192ffb7515839b1cc8457e0a8c1e09783de019d3
baseline version:
 linuxa811c1fa0a02c062555b54651065899437bacdbe

Last test of basis   150064  2020-05-07 06:00:34 Z1 days
Testing same since   150081  2020-05-07 23:40:11 Z0 days1 attempts


People who touched revisions under test:
  Alan Maguire 
  Catalin Marinas 
  Christian Borntraeger 
  Christoph Hellwig 
  Fangrui Song 
  James Morse 
  Jim Mattson 
  Kashyap Chamarthy 
  Linus Torvalds 
  Marc Zyngier 
  Mark Rutland 
  Masami 

[PATCH] xen/cpuhotplug: Fix initial CPU offlining for PV(H) guests

[Boris Ostrovsky]

Commit a926f81d2f6c ("xen/cpuhotplug: Replace cpu_up/down() with
device_online/offline()") replaced cpu_down() with device_offline()
call which requires that the CPU has been registered before. This
registration, however, happens later from topology_init() which
is called as subsys_initcall(). setup_vcpu_hotplug_event(), on the
other hand, is invoked earlier, during arch_initcall().

As result, booting a PV(H) guest with vcpus < maxvcpus causes a crash.

Move setup_vcpu_hotplug_event() (and therefore setup_cpu_watcher()) to
late_initcall(). In addition, instead of performing all offlining steps
in setup_cpu_watcher() simply call disable_hotplug_cpu().

Fixes: a926f81d2f6c (xen/cpuhotplug: Replace cpu_up/down() with 
device_online/offline()"
Signed-off-by: Boris Ostrovsky 
---
 drivers/xen/cpu_hotplug.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/xen/cpu_hotplug.c b/drivers/xen/cpu_hotplug.c
index ec975de..b96b11e 100644
--- a/drivers/xen/cpu_hotplug.c
+++ b/drivers/xen/cpu_hotplug.c
@@ -93,10 +93,8 @@ static int setup_cpu_watcher(struct notifier_block *notifier,
(void)register_xenbus_watch(_watch);
 
for_each_possible_cpu(cpu) {
-   if (vcpu_online(cpu) == 0) {
-   device_offline(get_cpu_device(cpu));
-   set_cpu_present(cpu, false);
-   }
+   if (vcpu_online(cpu) == 0)
+   disable_hotplug_cpu(cpu);
}
 
return NOTIFY_DONE;
@@ -119,5 +117,5 @@ static int __init setup_vcpu_hotplug_event(void)
return 0;
 }
 
-arch_initcall(setup_vcpu_hotplug_event);
+late_initcall(setup_vcpu_hotplug_event);
 
-- 
1.8.3.1

Re: [PATCH v8 12/12] x86/HVM: don't needlessly intercept APERF/MPERF/TSC MSR reads

[Andrew Cooper]

On 05/05/2020 09:20, Jan Beulich wrote:
> If the hardware can handle accesses, we should allow it to do so. This
> way we can expose EFRO to HVM guests,

I'm reminded now of the conversation I'm sure we've had before, although
I have a feeling it was on IRC.

APERF/MPERF (including the EFRO interface on AMD) are free running
counters but only in C0.  The raw values are not synchronised across
threads.

A vCPU which gets rescheduled has a 50% chance of finding the one or
both values going backwards, and a 100% chance of totally bogus calculation.

There is no point exposing APERF/MPERF to guests.  It can only be used
safely in hypervisor context, on AMD hardware with a CLGI/STGI region,
or on Intel hardware in an NMI handler if you trust that a machine check
isn't going to ruin your day.

VMs have no way of achieving the sampling requirements, and has a fair
chance of getting a plausible-but-wrong answer.

The only possibility to do this safely is on a VM which is pinned to
pCPU for its lifetime, but even I'm unconvinced of the correctness.

I don't think we should be exposing this functionality to guests at all,
although I might be persuaded if someone wanting to use it in a VM can
provide a concrete justification of why the above problems won't get in
their way.

~Andrew

[ovmf test] 150082: all pass - PUSHED

[osstest service owner]

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 3a3713e62cfad00d78bb938b0d9fb1eedaeff314
baseline version:
 ovmf 8293e6766a884918a6b608c64543caab49870597

Last test of basis   150063  2020-05-07 05:27:13 Z1 days
Testing same since   150082  2020-05-08 04:09:39 Z0 days1 attempts


People who touched revisions under test:
  Rebecca Cran 

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
   8293e6766a..3a3713e62c  3a3713e62cfad00d78bb938b0d9fb1eedaeff314 -> 
xen-tested-master

Re: [PATCH v8 10/12] x86/HVM: scale MPERF values reported to guests (on AMD)

[Andrew Cooper]

On 05/05/2020 09:18, Jan Beulich wrote:
> AMD's PM specifies that MPERF (and its r/o counterpart) reads are
> affected by the TSC ratio. Hence when processing such reads in software
> we too should scale the values. While we don't currently (yet) expose
> the underlying feature flags, besides us allowing the MSRs to be read
> nevertheless, RDPRU is going to expose the values even to user space.
>
> Furthermore, due to the not exposed feature flags, this change has the
> effect of making properly inaccessible (for reads) the two MSRs.
>
> Note that writes to MPERF (and APERF) continue to be unsupported.

Linux is now using MPERF/APERF for its frequency-invariant scheduling
logic.  Irritatingly, via its read/write alias rather than its read-only
alias.  Even more irritatingly, Intel's reference algorithm recommends
writing to both, despite this being being far less efficient than (one
of) AMD's (two) algorithm(s) which tells you just to subtract the values
you last sampled.

On the one hand, I'm tempted to suggest that we offer EFRO on Intel and
update Linux to use it.  OTOH, that would VMExit as Intel CPUs don't
understand the EFRO interface.

I can't see any sane way to virtualise the write behaviour for MPERF/APERF.

>
> Signed-off-by: Jan Beulich 
> ---
> v3: New.
> ---
> I did consider whether to put the code in guest_rdmsr() instead, but
> decided that it's better to have it next to TSC handling.

Please do put it in guest_rdmsr().  This is code hygene just as much as
bool_t or style fixes are.

The relationship to TSC is passing-at-best.

>
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -3478,6 +3478,22 @@ int hvm_msr_read_intercept(unsigned int
>  *msr_content = v->arch.hvm.msr_tsc_adjust;
>  break;
>  
> +case MSR_MPERF_RD_ONLY:
> +if ( !d->arch.cpuid->extd.efro )
> +{
> +goto gp_fault;
> +
> +case MSR_IA32_MPERF:
> +if ( !(d->arch.cpuid->basic.raw[6].c &
> +   CPUID6_ECX_APERFMPERF_CAPABILITY) )
> +goto gp_fault;
> +}
> +if ( rdmsr_safe(msr, *msr_content) )
> +goto gp_fault;
> +if ( d->arch.cpuid->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON) 
> )

I suspect we want to gain amd_like() outside of the emulator.

> +*msr_content = hvm_get_guest_tsc_fixed(v, *msr_content);
> +break;
> +
>  case MSR_APIC_BASE:
>  *msr_content = vcpu_vlapic(v)->hw.apic_base_msr;
>  break;
> --- a/xen/include/asm-x86/msr-index.h
> +++ b/xen/include/asm-x86/msr-index.h
> @@ -405,6 +405,9 @@
>  #define MSR_IA32_MPERF   0x00e7
>  #define MSR_IA32_APERF   0x00e8
>  
> +#define MSR_MPERF_RD_ONLY0xc0e7
> +#define MSR_APERF_RD_ONLY0xc0e8

S/RD_ONLY/RO/ ?  No loss of meaning.  Also, above the legacy line please.

~Andrew

> +
>  #define MSR_IA32_THERM_CONTROL   0x019a
>  #define MSR_IA32_THERM_INTERRUPT 0x019b
>  #define MSR_IA32_THERM_STATUS0x019c
>

[libvirt test] 150083: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-libvirt   6 libvirt-buildfail REGR. vs. 146182
 build-i386-libvirt6 libvirt-buildfail REGR. vs. 146182
 build-arm64-libvirt   6 libvirt-buildfail REGR. vs. 146182
 build-armhf-libvirt   6 libvirt-buildfail REGR. vs. 146182

Tests which did not succeed, but are not blocking:
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-amd64-libvirt-vhd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-xsm  1 build-check(1)   blocked  n/a

version targeted for testing:
 libvirt  23bf93884c6346206e87c0f14d93f905e8c81267
baseline version:
 libvirt  a1cd25b919509be2645dbe6f952d5263e0d4e4e5

Last test of basis   146182  2020-01-17 06:00:23 Z  112 days
Failing since146211  2020-01-18 04:18:52 Z  111 days  102 attempts
Testing same since   150083  2020-05-08 04:18:46 Z0 days1 attempts


People who touched revisions under test:
  Andrea Bolognani 
  Arnaud Patard 
  Artur Puzio 
  Bjoern Walk 
  Boris Fiuczynski 
  Chen Hanxiao 
  Christian Borntraeger 
  Christian Ehrhardt 
  Christian Schoenebeck 
  Collin Walling 
  Cornelia Huck 
  Daniel Henrique Barboza 
  Daniel P. Berrangé 
  Daniel Veillard 
  Dario Faggioli 
  Erik Skultety 
  Gaurav Agrawal 
  Han Han 
  Jamie Strandboge 
  Jim Fehlig 
  Jiri Denemark 
  Jonathon Jongsma 
  Julio Faracco 
  Ján Tomko 
  Laine Stump 
  Leonid Bloch 
  Lin Ma 
  Marc-André Lureau 
  Marek Marczykowski-Górecki 
  Mark Asselstine 
  Mauro S. M. Rodrigues 
  Michal Privoznik 
  Nikolay Shirokovskiy 
  Pavel Hrdina 
  Pavel Mores 
  Peter Krempa 
  Philipp Hahn 
  Pino Toscano 
  Prathamesh Chavan 
  Rafael Fonseca 
  Richard W.M. Jones 
  Rikard Falkeborn 
  Ryan Moeller 
  Sahid Orentino Ferdjaoui 
  Sebastian Mitterle 
  Seeteena Thoufeek 
  Stefan Berger 
  Stefan Berger 
  Stefan Hajnoczi 
  Thomas Huth 
  Tobin Feldman-Fitzthum 
  Wu Qingliang 
  Xu Yandong 
  Yi Li 
  Your Name 
  Zhang Bo 
  zhenwei pi 
  Zhimin Feng 

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  fail
 build-arm64-libvirt  fail
 build-armhf-libvirt  fail
 build-i386-libvirt   fail
 build-amd64-pvopspass
 build-arm64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   blocked 
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmblocked 
 test-amd64-amd64-libvirt-xsm blocked 
 test-arm64-arm64-libvirt-xsm blocked 
 test-amd64-i386-libvirt-xsm  blocked 
 test-amd64-amd64-libvirt blocked 
 test-arm64-arm64-libvirt blocked 
 test-armhf-armhf-libvirt blocked 
 test-amd64-i386-libvirt  blocked 
 test-amd64-amd64-libvirt-pairblocked 
 test-amd64-i386-libvirt-pair  

Re: [PATCH v8 09/12] x86emul: support FXSAVE/FXRSTOR

[Andrew Cooper]

On 05/05/2020 09:16, Jan Beulich wrote:
> Note that FPU selector handling as well as MXCSR mask saving for now
> does not honor differences between host and guest visible featuresets.
>
> While for Intel operation of the insns with CR4.OSFXSR=0 is
> implementation dependent, use the easiest solution there: Simply don't
> look at the bit in the first place. For AMD and alike the behavior is
> well defined, so it gets handled together with FFXSR.
>
> Signed-off-by: Jan Beulich 
> ---
> v8: Respect EFER.FFXSE and CR4.OSFXSR. Correct wrong X86EMUL_NO_*
> dependencies. Reduce #ifdef-ary.
> v7: New.
>
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -953,6 +958,29 @@ typedef union {
>  uint32_t data32[16];
>  } mmval_t;
>  
> +struct x86_fxsr {
> +uint16_t fcw;
> +uint16_t fsw;
> +uint16_t ftw:8, :8;

uint8_t

> +uint16_t fop;
> +union {
> +struct {
> +uint32_t offs;
> +uint32_t sel:16, :16;

uint16_t

> +};
> +uint64_t addr;
> +} fip, fdp;
> +uint32_t mxcsr;
> +uint32_t mxcsr_mask;
> +struct {
> +uint8_t data[10];
> +uint8_t _[6];

I'd be tempted to suggest uint16_t :16, :16, :16; here, which gets rid
of the named field, or explicitly rsvd to match below.

> +} fpreg[8];
> +uint64_t __attribute__ ((aligned(16))) xmm[16][2];
> +uint64_t _[6];

This field however is used below.  It would be clearer being named 'rsvd'.

> +uint64_t avl[6];
> +};
> +
>  /*
>   * While proper alignment gets specified above, this doesn't get honored by
>   * the compiler for automatic variables. Use this helper to instantiate a
> @@ -1910,6 +1938,7 @@ amd_like(const struct x86_emulate_ctxt *
>  #define vcpu_has_cmov()(ctxt->cpuid->basic.cmov)
>  #define vcpu_has_clflush() (ctxt->cpuid->basic.clflush)
>  #define vcpu_has_mmx() (ctxt->cpuid->basic.mmx)
> +#define vcpu_has_fxsr()(ctxt->cpuid->basic.fxsr)
>  #define vcpu_has_sse() (ctxt->cpuid->basic.sse)
>  #define vcpu_has_sse2()(ctxt->cpuid->basic.sse2)
>  #define vcpu_has_sse3()(ctxt->cpuid->basic.sse3)
> @@ -8125,6 +8154,47 @@ x86_emulate(
>  case X86EMUL_OPC(0x0f, 0xae): case X86EMUL_OPC_66(0x0f, 0xae): /* Grp15 
> */
>  switch ( modrm_reg & 7 )
>  {
> +#if !defined(X86EMUL_NO_FPU) || !defined(X86EMUL_NO_MMX) || \
> +!defined(X86EMUL_NO_SIMD)
> +case 0: /* fxsave */
> +case 1: /* fxrstor */
> +generate_exception_if(vex.pfx, EXC_UD);
> +vcpu_must_have(fxsr);
> +generate_exception_if(ea.type != OP_MEM, EXC_UD);
> +generate_exception_if(!is_aligned(ea.mem.seg, ea.mem.off, 16,
> +  ctxt, ops),
> +  EXC_GP, 0);
> +fail_if(!ops->blk);
> +op_bytes =
> +#ifdef __x86_64__
> +!mode_64bit() ? offsetof(struct x86_fxsr, xmm[8]) :
> +#endif
> +sizeof(struct x86_fxsr);
> +if ( amd_like(ctxt) )
> +{
> +if ( !ops->read_cr ||
> + ops->read_cr(4, , ctxt) != X86EMUL_OKAY )
> +cr4 = X86_CR4_OSFXSR;

Why do we want to assume OSFXSR in the case of a read_cr() failure,
rather than bailing on the entire instruction?

> +if ( !ops->read_msr ||
> + ops->read_msr(MSR_EFER, _val, ctxt) != X86EMUL_OKAY 
> )
> +msr_val = 0;
> +if ( !(cr4 & X86_CR4_OSFXSR) ||
> + (mode_64bit() && mode_ring0() && (msr_val & 
> EFER_FFXSE)) )
> +op_bytes = offsetof(struct x86_fxsr, xmm[0]);

This is a very peculiar optimisation in AMD processors...  (but your
logic here does agree with the description AFAICT)

> +}
> +/*
> + * This could also be X86EMUL_FPU_mmx, but it shouldn't be
> + * X86EMUL_FPU_xmm, as we don't want CR4.OSFXSR checked.
> + */
> +get_fpu(X86EMUL_FPU_fpu);
> +state->blk = modrm_reg & 1 ? blk_fxrstor : blk_fxsave;
> +if ( (rc = ops->blk(ea.mem.seg, ea.mem.off, NULL,
> +sizeof(struct x86_fxsr), &_regs.eflags,
> +state, ctxt)) != X86EMUL_OKAY )
> +goto done;
> +break;
> +#endif /* X86EMUL_NO_{FPU,MMX,SIMD} */
> +
>  #ifndef X86EMUL_NO_SIMD
>  case 2: /* ldmxcsr */
>  generate_exception_if(vex.pfx, EXC_UD);
> @@ -11611,6 +11681,8 @@ int x86_emul_blk(
>  struct x86_emulate_state *state,
>  struct x86_emulate_ctxt *ctxt)
>  {
> +int rc = X86EMUL_OKAY;
> +
>  switch ( state->blk )
>  {
>  bool zf;
> @@ -11819,6 +11891,77 @@ int x86_emul_blk(
>  
>  #endif /* X86EMUL_NO_FPU */
>  
> +#if !defined(X86EMUL_NO_FPU) || !defined(X86EMUL_NO_MMX) || \
> +

Re: [PATCH v8 08/12] x86emul: support FLDENV and FRSTOR

[Andrew Cooper]

On 08/05/2020 16:04, Jan Beulich wrote:
>>> +}
>>> +
>>> +if ( bytes == sizeof(fpstate.env) )
>>> +ptr = NULL;
>>> +else
>>> +ptr += sizeof(fpstate.env);
>>> +break;
>>> +
>>> +case sizeof(struct x87_env16):
>>> +case sizeof(struct x87_env16) + sizeof(fpstate.freg):
>>> +{
>>> +const struct x87_env16 *env = ptr;
>>> +
>>> +fpstate.env.fcw = env->fcw;
>>> +fpstate.env.fsw = env->fsw;
>>> +fpstate.env.ftw = env->ftw;
>>> +
>>> +if ( state->rex_prefix )
>>> +{
>>> +fpstate.env.mode.prot.fip = env->mode.prot.fip;
>>> +fpstate.env.mode.prot.fcs = env->mode.prot.fcs;
>>> +fpstate.env.mode.prot.fdp = env->mode.prot.fdp;
>>> +fpstate.env.mode.prot.fds = env->mode.prot.fds;
>>> +fpstate.env.mode.prot.fop = 0; /* unknown */
>>> +}
>>> +else
>>> +{
>>> +unsigned int fip = env->mode.real.fip_lo +
>>> +   (env->mode.real.fip_hi << 16);
>>> +unsigned int fdp = env->mode.real.fdp_lo +
>>> +   (env->mode.real.fdp_hi << 16);
>>> +unsigned int fop = env->mode.real.fop;
>>> +
>>> +fpstate.env.mode.prot.fip = fip & 0xf;
>>> +fpstate.env.mode.prot.fcs = fip >> 4;
>>> +fpstate.env.mode.prot.fop = fop;
>>> +fpstate.env.mode.prot.fdp = fdp & 0xf;
>>> +fpstate.env.mode.prot.fds = fdp >> 4;
>> This looks mostly the same as the translation done above, so maybe
>> could be abstracted anyway in a macro to avoid the code repetition?
>> (ie: fpstate_real_to_prot(src, dst) or some such).
> Just the 5 assignments could be put in an inline function, but
> if we also wanted to abstract away the declarations with their
> initializers, it would need to be a macro because of the
> different types of fpstate.env and *env. While I'd generally
> prefer inline functions, the macro would have the benefit that
> it could be #define-d / #undef-d right inside this case block.
> Thoughts?

Code like this is large in terms of volume, but it is completely crystal
clear (with the requested comments in place) and easy to follow.

I don't see how attempting to abstract out two small portions is going
to be an improvement.

~Andrew

Re: [PATCH v8 08/12] x86emul: support FLDENV and FRSTOR

[Andrew Cooper]

On 05/05/2020 09:16, Jan Beulich wrote:
> While the Intel SDM claims that FRSTOR itself may raise #MF upon
> completion, this was confirmed by Intel to be a doc error which will be
> corrected in due course; behavior is like FLDENV, and like old hard copy
> manuals describe it. Otherwise we'd have to emulate the insn by filling
> st(N) in suitable order, followed by FLDENV.

I wouldn't bother calling this out at all.  We know the doc is going to
be corrected in the next revision, and "what we would have done if the
docs error was in fact accurate" only adds confusion to an already
complicated change.

> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -857,6 +857,7 @@ struct x86_emulate_state {
>  blk_NONE,
>  blk_enqcmd,
>  #ifndef X86EMUL_NO_FPU
> +blk_fld, /* FLDENV, FRSTOR */
>  blk_fst, /* FNSTENV, FNSAVE */
>  #endif
>  blk_movdir,
> @@ -4948,21 +4949,14 @@ x86_emulate(
>  dst.bytes = 4;
>  emulate_fpu_insn_memdst(b, modrm_reg & 7, dst.val);
>  break;
> -case 4: /* fldenv - TODO */
> -state->fpu_ctrl = true;
> -goto unimplemented_insn;
> -case 5: /* fldcw m2byte */
> -state->fpu_ctrl = true;
> -fpu_memsrc16:
> -if ( (rc = ops->read(ea.mem.seg, ea.mem.off, ,
> - 2, ctxt)) != X86EMUL_OKAY )
> -goto done;
> -emulate_fpu_insn_memsrc(b, modrm_reg & 7, src.val);
> -break;

The commit message should however note that the larger-than-expected
diff is purely code motion for case 5, to arrange fldenv to fall though
into fstenv.

Alternatively, could be pulled out into an earlier patch.

> +case 4: /* fldenv */
> +/* Raise #MF now if there are pending unmasked exceptions. */
> +emulate_fpu_insn_stub(0xd9, 0xd0 /* fnop */);
> +/* fall through */
>  case 6: /* fnstenv */
>  fail_if(!ops->blk);
> -state->blk = blk_fst;
> -/* REX is meaningless for this insn by this point. */
> +state->blk = modrm_reg & 2 ? blk_fst : blk_fld;
> +/* REX is meaningless for these insns by this point. */
>  rex_prefix = in_protmode(ctxt, ops);
>  if ( (rc = ops->blk(ea.mem.seg, ea.mem.off, NULL,
>  op_bytes > 2 ? sizeof(struct x87_env32)
> @@ -4972,6 +4966,14 @@ x86_emulate(
>  goto done;
>  state->fpu_ctrl = true;
>  break;
> +case 5: /* fldcw m2byte */
> +state->fpu_ctrl = true;
> +fpu_memsrc16:
> +if ( (rc = ops->read(ea.mem.seg, ea.mem.off, ,
> + 2, ctxt)) != X86EMUL_OKAY )
> +goto done;
> +emulate_fpu_insn_memsrc(b, modrm_reg & 7, src.val);
> +break;
>  case 7: /* fnstcw m2byte */
>  state->fpu_ctrl = true;
>  fpu_memdst16:
> @@ -5124,13 +5126,14 @@ x86_emulate(
>  dst.bytes = 8;
>  emulate_fpu_insn_memdst(b, modrm_reg & 7, dst.val);
>  break;
> -case 4: /* frstor - TODO */
> -state->fpu_ctrl = true;
> -goto unimplemented_insn;
> +case 4: /* frstor */
> +/* Raise #MF now if there are pending unmasked exceptions. */
> +emulate_fpu_insn_stub(0xd9, 0xd0 /* fnop */);
> +/* fall through */
>  case 6: /* fnsave */
>  fail_if(!ops->blk);
> -state->blk = blk_fst;
> -/* REX is meaningless for this insn by this point. */
> +state->blk = modrm_reg & 2 ? blk_fst : blk_fld;
> +/* REX is meaningless for these insns by this point. */
>  rex_prefix = in_protmode(ctxt, ops);
>  if ( (rc = ops->blk(ea.mem.seg, ea.mem.off, NULL,
>  op_bytes > 2 ? sizeof(struct x87_env32) 
> + 80
> @@ -11648,6 +11651,89 @@ int x86_emul_blk(
>  
>  #ifndef X86EMUL_NO_FPU
>  
> +case blk_fld:
> +ASSERT(!data);
> +
> +/* state->rex_prefix carries CR0.PE && !EFLAGS.VM setting */
> +switch ( bytes )
> +{
> +case sizeof(fpstate.env):
> +case sizeof(fpstate):
> +memcpy(, ptr, sizeof(fpstate.env));
> +if ( !state->rex_prefix )
> +{
> +unsigned int fip = fpstate.env.mode.real.fip_lo +
> +   (fpstate.env.mode.real.fip_hi << 16);
> +unsigned int fdp = fpstate.env.mode.real.fdp_lo +
> +   (fpstate.env.mode.real.fdp_hi << 16);

Re: [PATCH v8 07/12] x86emul: support FNSTENV and FNSAVE

[Andrew Cooper]

On 05/05/2020 09:15, Jan Beulich wrote:
> To avoid introducing another boolean into emulator state, the
> rex_prefix field gets (ab)used to convey the real/VM86 vs protected mode
> info (affecting structure layout, albeit not size) to x86_emul_blk().
>
> Signed-off-by: Jan Beulich 
> ---
> TBD: The full 16-bit padding fields in the 32-bit structures get filled
>  with all ones by modern CPUs (i.e. other than the comment says for

You really mean "unlike" here, rather than "other".  They do not have
the same meaning in this context.

(I think you're also missing a "what", which I'm guessing is just an
oversight.)

>  FIP and FDP). We may want to mirror this as well (for the real mode
>  variant), even if those fields' contents are unspecified.

This is surprising behaviour, but I expect it dates back to external x87
processors and default MMIO behaviour.

If it is entirely consistent, it match be nice to match.  OTOH, the
manuals are very clear that it is reserved, which I think gives us the
liberty to use the easier implementation.

> ---
> v7: New.
>
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -897,6 +900,50 @@ struct x86_emulate_state {
>  #define PTR_POISON NULL /* 32-bit builds are for user-space, so NULL is OK. 
> */
>  #endif
>  
> +#ifndef X86EMUL_NO_FPU
> +struct x87_env16 {
> +uint16_t fcw;
> +uint16_t fsw;
> +uint16_t ftw;
> +union {
> +struct {
> +uint16_t fip_lo;
> +uint16_t fop:11, :1, fip_hi:4;
> +uint16_t fdp_lo;
> +uint16_t :12, fdp_hi:4;
> +} real;
> +struct {
> +uint16_t fip;
> +uint16_t fcs;
> +uint16_t fdp;
> +uint16_t fds;
> +} prot;
> +} mode;
> +};
> +
> +struct x87_env32 {
> +uint32_t fcw:16, :16;
> +uint32_t fsw:16, :16;
> +uint32_t ftw:16, :16;

uint16_t fcw, :16;
uint16_t fsw, :16;
uint16_t ftw, :16;

which reduces the number of 16 bit bitfields.

> +union {
> +struct {
> +/* some CPUs/FPUs also store the full FIP here */
> +uint32_t fip_lo:16, :16;
> +uint32_t fop:11, :1, fip_hi:16, :4;
> +/* some CPUs/FPUs also store the full FDP here */
> +uint32_t fdp_lo:16, :16;
> +uint32_t :12, fdp_hi:16, :4;

Annoyingly, two of these lines can't use uint16_t.  I'm torn as to
whether to suggest converting the other two which can.

> +} real;
> +struct {
> +uint32_t fip;
> +uint32_t fcs:16, fop:11, :5;
> +uint32_t fdp;
> +uint32_t fds:16, :16;

These two can be converted safely.

> @@ -4912,9 +4959,19 @@ x86_emulate(
>  goto done;
>  emulate_fpu_insn_memsrc(b, modrm_reg & 7, src.val);
>  break;
> -case 6: /* fnstenv - TODO */
> +case 6: /* fnstenv */
> +fail_if(!ops->blk);
> +state->blk = blk_fst;
> +/* REX is meaningless for this insn by this point. */
> +rex_prefix = in_protmode(ctxt, ops);

Code like this is why I have such a strong objection to obfuscating macros.

It reads as if you're updating a local variable, alongside a comment
explaining that it is meaningless.

It is critically important for clarity that the comment state that
you're (ab)using the field to pass information into ->blk(), and I'd go
so far as suggesting

/*state->*/rex_prefix = in_protmode(ctxt, ops);

to reinforce the point that rex_prefix isn't a local variable, seeing
the obfuscation prevents a real state->rex_prefix from working.

> +if ( (rc = ops->blk(ea.mem.seg, ea.mem.off, NULL,
> +op_bytes > 2 ? sizeof(struct x87_env32)
> + : sizeof(struct x87_env16),
> +&_regs.eflags,
> +state, ctxt)) != X86EMUL_OKAY )
> +goto done;
>  state->fpu_ctrl = true;
> -goto unimplemented_insn;
> +break;
>  case 7: /* fnstcw m2byte */
>  state->fpu_ctrl = true;
>  fpu_memdst16:
> @@ -5068,9 +5125,21 @@ x86_emulate(
>  emulate_fpu_insn_memdst(b, modrm_reg & 7, dst.val);
>  break;
>  case 4: /* frstor - TODO */
> -case 6: /* fnsave - TODO */
>  state->fpu_ctrl = true;
>  goto unimplemented_insn;
> +case 6: /* fnsave */
> +fail_if(!ops->blk);
> +state->blk = blk_fst;
> +/* REX is meaningless for this insn by this point. */
> +rex_prefix = in_protmode(ctxt, ops);
> +if ( (rc = ops->blk(ea.mem.seg, ea.mem.off, NULL,
> +op_bytes > 2 ? 

[qemu-mainline test] 150077: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-i386-xsm6 xen-buildfail REGR. vs. 150061

Tests which did not succeed, but are not blocking:
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  1 build-check(1)  blocked n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-xsm1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10   fail  like 150061
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 150061
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 150061
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 150061
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 150061
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 150061
 test-amd64-i386-xl-pvshim12 guest-start  fail   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-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-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-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 14 saverestore-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-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-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-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-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-xl-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-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass

version targeted for testing:
 qemuu3c7adbc67d9a5c3e992a4dd13b8704464daaad5b
baseline version:
 qemuu570a9214827e3d42f7173c4d4c9f045b99834cf0

Last test of basis   150061  2020-05-06 22:06:59 Z1 days
Testing same since   150077  2020-05-07 15:37:29 Z1 days1 attempts


People who touched revisions under test:
  Alexey Kardashevskiy 
  Alexey Krasikov 
  Chen Qun 
  Cornelia Huck 
  Cédric Le Goater 
  Daniel Henrique Barboza 
  Daniel P. Berrangé 
  Daniele Buono 
  David Gibson 
  Eric Auger 
  Greg Kurz 
  Maxim Levitsky 
  Nicholas Piggin 
  Peter 

Re: [PATCH] x86/idle: prevent entering C6 with in service interrupts on Intel

[Roger Pau Monné]

On Fri, May 08, 2020 at 03:46:08PM +0200, Jan Beulich wrote:
> On 07.05.2020 15:22, Roger Pau Monne wrote:
> > diff --git a/xen/arch/x86/acpi/cpu_idle.c b/xen/arch/x86/acpi/cpu_idle.c
> > index b83446e77d..5023fea148 100644
> > --- a/xen/arch/x86/acpi/cpu_idle.c
> > +++ b/xen/arch/x86/acpi/cpu_idle.c
> > @@ -573,6 +573,25 @@ static bool errata_c6_eoi_workaround(void)
> >  return (fix_needed && cpu_has_pending_apic_eoi());
> >  }
> >  
> > +static int8_t __read_mostly disable_c6_isr = -1;
> > +boolean_param("disable-c6-isr", disable_c6_isr);
> > +
> > +/*
> > + * Errata CLX30: A Pending Fixed Interrupt May Be Dispatched Before an
> > + * Interrupt of The Same Priority Completes.
> > + *
> > + * Prevent entering C6 if there are pending lapic interrupts, or else the
> > + * processor might dispatch further pending interrupts before the first 
> > one has
> > + * been completed.
> > + */
> > +bool errata_c6_isr_workaround(void)
> > +{
> > +if ( unlikely(disable_c6_isr == -1) )
> > +disable_c6_isr = boot_cpu_data.x86_vendor == X86_VENDOR_INTEL;
> > +
> > +return disable_c6_isr && cpu_has_pending_apic_eoi();
> 
> This check being the same as in errata_c6_eoi_workaround(),
> why don't you simply extend that function? (See also below.)
> Also both variable and command line option descriptor could
> go inside the function, to limit their scopes.

Since this is actually a superset (as it covers all Intel CPUs), I
should delete the previous (more restricted) workaround matching
logic.

> > @@ -676,7 +695,8 @@ static void acpi_processor_idle(void)
> >  return;
> >  }
> >  
> > -if ( (cx->type == ACPI_STATE_C3) && errata_c6_eoi_workaround() )
> > +if ( (cx->type == ACPI_STATE_C3) &&
> > + (errata_c6_eoi_workaround() || errata_c6_isr_workaround()) )
> >  cx = power->safe_state;
> 
> I realize you only add to existing code, but I'm afraid this
> existing code cannot be safely added to. Already prior to
> your change there was a curious mismatch of C3 and c6 on this
> line, and I don't see how ACPI_STATE_C3 correlates with
> "core C6" state. Now this may have been the convention for
> Nehalem/Westmere systems, but already the mwait-idle entries
> for these CPU models have 4 entries (albeit such that they
> match this scheme). As a result I think this at the very
> least needs to be >=, not ==, even more so now that you want
> to cover all Intel CPUs.

Hm, I think this is because AFAICT acpi_processor_idle only
understands up to ACPI_STATE_C3, passing a type > ACPI_STATE_C3 will
just cause it to fallback to C0. I've adjusted the comparison to use
>= instead, as a safety imporvement in case we add support for more
states to acpi_processor_idle.

> Obviously (I think) it is a mistake that mwait-idle doesn't
> also activate this workaround, which imo is another reason to
> stick to just errata_c6_eoi_workaround().

I assumed the previous workaround didn't apply to any of the CPUs
supported by the mwait-idle driver, since it seems to only support
recent-ish models.

> > --- a/xen/arch/x86/cpu/mwait-idle.c
> > +++ b/xen/arch/x86/cpu/mwait-idle.c
> > @@ -770,6 +770,9 @@ static void mwait_idle(void)
> > return;
> > }
> >  
> > +   if (cx->type == ACPI_STATE_C3 && errata_c6_isr_workaround())
> > +   cx = power->safe_state;
> 
> Here it becomes even more relevant I think that == not be
> used, as the static tables list deeper C-states; it's just
> that the SKX table, which also gets used for CLX afaict, has
> no entries beyond C6-SKX. Others with deeper ones presumably
> have the deeper C-states similarly affected (or not) by this
> erratum.
> 
> For reference, mwait_idle_cpu_init() has
> 
>   hint = flg2MWAIT(cpuidle_state_table[cstate].flags);
>   state = MWAIT_HINT2CSTATE(hint) + 1;
> ...
>   cx->type = state;
> 
> i.e. the value you compare against is derived from the static
> table entries. For Nehalem/Westmere this means that what goes
> under ACPI_STATE_C3 is indeed C6-NHM, and this looks to match
> for all the non-Atoms, but for none of the Atoms. Now Atoms
> could easily be unaffected, but (just to take an example) if
> C6-SNB was affected, surely C7-SNB would be, too.

Yes, I've adjusted this to use cx->type >= 3 instead. I have to admit
I'm confused by the name of the states being C6-* while the cstate
value reported by Xen will be 3 (I would instead expect the type to be
6 in order to match the name).

Thanks, Roger.

Re: [PATCH v8 08/12] x86emul: support FLDENV and FRSTOR

[Roger Pau Monné]

On Fri, May 08, 2020 at 05:04:02PM +0200, Jan Beulich wrote:
> On 08.05.2020 15:37, Roger Pau Monné wrote:
> > On Tue, May 05, 2020 at 10:16:20AM +0200, Jan Beulich wrote:
> >> --- a/tools/tests/x86_emulator/test_x86_emulator.c
> >> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
> >> @@ -11648,6 +11651,89 @@ int x86_emul_blk(
> >>  
> >>  #ifndef X86EMUL_NO_FPU
> >>  
> >> +case blk_fld:
> >> +ASSERT(!data);
> >> +
> >> +/* state->rex_prefix carries CR0.PE && !EFLAGS.VM setting */
> >> +switch ( bytes )
> >> +{
> >> +case sizeof(fpstate.env):
> >> +case sizeof(fpstate):
> >> +memcpy(, ptr, sizeof(fpstate.env));
> >> +if ( !state->rex_prefix )
> >> +{
> >> +unsigned int fip = fpstate.env.mode.real.fip_lo +
> >> +   (fpstate.env.mode.real.fip_hi << 16);
> >> +unsigned int fdp = fpstate.env.mode.real.fdp_lo +
> >> +   (fpstate.env.mode.real.fdp_hi << 16);
> >> +unsigned int fop = fpstate.env.mode.real.fop;
> >> +
> >> +fpstate.env.mode.prot.fip = fip & 0xf;
> >> +fpstate.env.mode.prot.fcs = fip >> 4;
> >> +fpstate.env.mode.prot.fop = fop;
> >> +fpstate.env.mode.prot.fdp = fdp & 0xf;
> >> +fpstate.env.mode.prot.fds = fdp >> 4;
> > 
> > I've found the layouts in the SDM vol. 1, but I haven't been able to
> > found the translation mechanism from real to protected. Could you
> > maybe add a reference here?
> 
> A reference to some piece of documentation? I don't think this
> is spelled out anywhere. It's also only one of various possible
> ways of doing the translation, but among them the most flexible
> one for possible consumers of the data (because of using the
> smallest possible offsets into the segments).

Having this written down as a comment would help, but maybe that's
just because I'm not familiar at all with all this stuff.

Again, likely a very stupid question, but I would expect:

fpstate.env.mode.prot.fip = fip;

Without the mask.

> >> +}
> >> +
> >> +if ( bytes == sizeof(fpstate.env) )
> >> +ptr = NULL;
> >> +else
> >> +ptr += sizeof(fpstate.env);
> >> +break;
> >> +
> >> +case sizeof(struct x87_env16):
> >> +case sizeof(struct x87_env16) + sizeof(fpstate.freg):
> >> +{
> >> +const struct x87_env16 *env = ptr;
> >> +
> >> +fpstate.env.fcw = env->fcw;
> >> +fpstate.env.fsw = env->fsw;
> >> +fpstate.env.ftw = env->ftw;
> >> +
> >> +if ( state->rex_prefix )
> >> +{
> >> +fpstate.env.mode.prot.fip = env->mode.prot.fip;
> >> +fpstate.env.mode.prot.fcs = env->mode.prot.fcs;
> >> +fpstate.env.mode.prot.fdp = env->mode.prot.fdp;
> >> +fpstate.env.mode.prot.fds = env->mode.prot.fds;
> >> +fpstate.env.mode.prot.fop = 0; /* unknown */
> >> +}
> >> +else
> >> +{
> >> +unsigned int fip = env->mode.real.fip_lo +
> >> +   (env->mode.real.fip_hi << 16);
> >> +unsigned int fdp = env->mode.real.fdp_lo +
> >> +   (env->mode.real.fdp_hi << 16);
> >> +unsigned int fop = env->mode.real.fop;
> >> +
> >> +fpstate.env.mode.prot.fip = fip & 0xf;
> >> +fpstate.env.mode.prot.fcs = fip >> 4;
> >> +fpstate.env.mode.prot.fop = fop;
> >> +fpstate.env.mode.prot.fdp = fdp & 0xf;
> >> +fpstate.env.mode.prot.fds = fdp >> 4;
> > 
> > This looks mostly the same as the translation done above, so maybe
> > could be abstracted anyway in a macro to avoid the code repetition?
> > (ie: fpstate_real_to_prot(src, dst) or some such).
> 
> Just the 5 assignments could be put in an inline function, but
> if we also wanted to abstract away the declarations with their
> initializers, it would need to be a macro because of the
> different types of fpstate.env and *env. While I'd generally
> prefer inline functions, the macro would have the benefit that
> it could be #define-d / #undef-d right inside this case block.
> Thoughts? 

I think a macro would be fine IMO.

Thanks, Roger.

Re: [PATCH] x86/PVH: PHYSDEVOP_pci_mmcfg_reserved should not blindly register a region

[Roger Pau Monné]

On Fri, May 08, 2020 at 05:11:35PM +0200, Jan Beulich wrote:
> On 08.05.2020 17:03, Roger Pau Monné wrote:
> > On Fri, May 08, 2020 at 02:43:38PM +0200, Jan Beulich wrote:
> >> --- a/xen/arch/x86/hvm/io.c
> >> +++ b/xen/arch/x86/hvm/io.c
> >> @@ -558,6 +558,47 @@ int register_vpci_mmcfg_handler(struct d
> >>  return 0;
> >>  }
> >>  
> >> +int unregister_vpci_mmcfg_handler(struct domain *d, paddr_t addr,
> >> +  unsigned int start_bus, unsigned int 
> >> end_bus,
> >> +  unsigned int seg)
> >> +{
> >> +struct hvm_mmcfg *mmcfg;
> >> +int rc = -ENOENT;
> >> +
> >> +ASSERT(is_hardware_domain(d));
> >> +
> >> +if ( start_bus > end_bus )
> >> +return -EINVAL;
> >> +
> >> +write_lock(>arch.hvm.mmcfg_lock);
> >> +
> >> +list_for_each_entry ( mmcfg, >arch.hvm.mmcfg_regions, next )
> >> +if ( mmcfg->addr == addr + (start_bus << 20) &&
> >> + mmcfg->segment == seg &&
> >> + mmcfg->start_bus == start_bus &&
> >> + mmcfg->size == ((end_bus - start_bus + 1) << 20) )
> >> +{
> >> +list_del(>next);
> >> +if ( !list_empty(>arch.hvm.mmcfg_regions) )
> >> +xfree(mmcfg);
> >> +else
> >> +{
> >> +/*
> >> + * Cannot unregister the MMIO handler - leave a fake entry
> >> + * on the list.
> >> + */
> >> +memset(mmcfg, 0, sizeof(*mmcfg));
> >> +list_add(>next, >arch.hvm.mmcfg_regions);
> > 
> > Instead of leaving this zombie entry around maybe we could add a
> > static bool in register_vpci_mmcfg_handler to signal whether the MMIO
> > intercept has been registered?
> 
> That was my initial plan indeed, but registration is per-domain.

Indeed, this would work now because it's only used by the hardware
domain, but it's not a good move long term.

What about splitting the registration into a
register_vpci_mmio_handler and call it from hvm_domain_initialise
like it's done for register_vpci_portio_handler?

That might be cleaner long term, sorry if it's more work.

> >> --- a/xen/arch/x86/physdev.c
> >> +++ b/xen/arch/x86/physdev.c
> >> @@ -559,12 +559,18 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H
> >>  if ( !ret && has_vpci(currd) )
> >>  {
> >>  /*
> >> - * For HVM (PVH) domains try to add the newly found MMCFG to 
> >> the
> >> - * domain.
> >> + * For HVM (PVH) domains try to add/remove the reported MMCFG
> >> + * to/from the domain.
> >>   */
> >> -ret = register_vpci_mmcfg_handler(currd, info.address,
> >> -  info.start_bus, 
> >> info.end_bus,
> >> -  info.segment);
> >> +if ( info.flags & XEN_PCI_MMCFG_RESERVED )
> > 
> > Do you think you could also add a small note in physdev.h regarding
> > the fact that XEN_PCI_MMCFG_RESERVED is used to register a MMCFG
> > region, and not setting it would imply an unregister request?
> > 
> > It's not obvious to me from the name of the flag.
> 
> The main purpose of the flag is to identify whether a region can be
> used (because of having been found marked suitably reserved by
> firmware). The flag not set effectively means "region is not marked
> reserved".

Looking at pci_mmcfg_arch_disable, should the region then also be
removed from mmio_ro_ranges? (kind of tangential to this patch)

> You pointing this out makes me wonder whether instead I
> should simply expand the if() in context, without making it behave
> like unregistration. Then again we'd have no way to unregister a
> region, and hence (ab)using this function for this purpose seems to
> makes sense (and, afaict, not require any code changes elsewhere).

Right now the only user I know of PHYSDEVOP_pci_mmcfg_reserved is
Linux, and AFAICT it always sets the XEN_PCI_MMCFG_RESERVED flag (at
least upstream).

I don't mind that much what we end up doing, as long as it's
documented in physdev.h. There's no documentation of that physdevop
hypercall at all, so if we provide proper documentation I would be
fine with treating a call with no flags as an unregistration request
(which is kind of what we already do for a classic PV hardware
domain).

Thanks, Roger.

Re: Xen Coding style

[Jürgen Groß]

On 08.05.20 17:50, Julien Grall wrote:



On 08/05/2020 15:42, Jürgen Groß wrote:

On 08.05.20 16:23, Tamas K Lengyel wrote:

On Fri, May 8, 2020 at 8:18 AM Jürgen Groß  wrote:


On 08.05.20 14:55, Tamas K Lengyel wrote:

On Fri, May 8, 2020 at 6:21 AM Julien Grall  wrote:


Hi Jan,

On 08/05/2020 12:20, Jan Beulich wrote:

On 08.05.2020 12:00, Julien Grall wrote:

You seem to be the maintainer with the most unwritten rules. Would
you mind to have a try at writing a coding style based on it?


On the basis that even small, single aspect patches to CODING_STYLE
have been ignored [1],


Your thread is one of the example why I started this thread. 
Agreeing on

specific rule doesn't work because it either result to bikesheding or
there is not enough interest to review rule by rule.


I don't think this would be a good use of my
time.


I would have assumed that the current situation (i.e
nitpicking/bikeshedding on the ML) is not a good use of your time :).

I would be happy to put some effort to help getting the coding style
right, however I believe focusing on an overall coding style would 
value

everyone's time better than a rule by rule discussion.


If I was promised (reasonable) feedback, I could take what I
have and try to add at least a few more things based on what I find
myself commenting on more frequently. But really I'd prefer it to
be done the other way around - for people to look at the patches
already sent, and for me to only subsequently send more. After all,
if already those adjustments are controversial, I don't think we
could settle on others.
While I understand this requires another investment from your 
part, I am
afraid it is going to be painful for someone else to go through 
all the

existing coding style bikeshedding and infer your unwritten rules.

It might be more beneficial for that person to pursue the work 
done by
Tamas and Viktor in the past (see my previous e-mail). This may 
mean to

adopt an existing coding style (BSD) and then tweak it.


Thanks Julien for restarting this discussion. IMHO agreeing on a set
of style rules ahead and then applying universally all at once is not
going to be productive since we are so all over the place. Instead, I
would recommend we start piece-by-piece. We introduce a baseline style
checker, then maintainers can decide when and if they want to move
their code-base to be under the automated style checker. That way we
have a baseline and each maintainer can decide on their own term when
they want to have their files be also style checked and in what form.
The upside of this route I think is pretty clear: we can have at least
partial automation even while we figure out what to do with some of
the more problematic files and quirks that are in our code-base. I
would highly prefer this route since I would immediately bring all
files I maintain over to the automated checker just so I never ever
have to deal with this again manually. What style is in use to me
really doesn't matter, BSD was very close with some minor tweaks, or
even what we use to check the style just as long as we have
_something_.


Wouldn't it make more sense to have a patch checker instead and accept
only patches which change code according to the style guide? This
wouldn't require to change complete files at a time.


So what are you going to do if a new contributor is unfortunate enough 
to modify code that doesn't pass the coding style checker? Are we going 
to impose him/her to fix the coding style before submitting his patch?


The modified portions should comply to the coding style. Same as in the
Linux kernel.





In theory, yes. But in practice this would require that we can agree
on a style that applies to all patches that touch any file within Xen.
We can't seem to do that because there are too many exceptions and
corner-cases and personal-preferences of maintainers that apply only
to a subset of the codebase. So AFAICT what you propose doesn't seem
to be a viable way to start.


I think long ago we already agreed to have a control file which tells a
style checker which style to apply (if any). As a start we could have a
patch checker checking only the commit message (has it a Signed-off-by:
etc.). The next step would be to add the control file, and the framework
to split the patch into the changed file hunks and passing each hunk to
the correct checking script (might be doing nothing in the beginning).
And then we could add some logic to the single checkers.


Yes the framework can be written down now (patches are welcomed). But 
this doesn't resolve the underlying problem. Aside files imported from 
Linux, what is the coding style of Xen?


The checker for Xen style needs to be written, yes.

The best way to describe it at the moment is a collection of unwritten 
rules that can differ even between maintainers of the same component. I 
don't really see how we could write a style checker based on this.


The style checker's rules must be written down, of course.

And 

Re: Xen Coding style

[Julien Grall]

On 08/05/2020 15:42, Jürgen Groß wrote:

On 08.05.20 16:23, Tamas K Lengyel wrote:

On Fri, May 8, 2020 at 8:18 AM Jürgen Groß  wrote:


On 08.05.20 14:55, Tamas K Lengyel wrote:

On Fri, May 8, 2020 at 6:21 AM Julien Grall  wrote:


Hi Jan,

On 08/05/2020 12:20, Jan Beulich wrote:

On 08.05.2020 12:00, Julien Grall wrote:

You seem to be the maintainer with the most unwritten rules. Would
you mind to have a try at writing a coding style based on it?


On the basis that even small, single aspect patches to CODING_STYLE
have been ignored [1],


Your thread is one of the example why I started this thread. 
Agreeing on

specific rule doesn't work because it either result to bikesheding or
there is not enough interest to review rule by rule.


I don't think this would be a good use of my
time.


I would have assumed that the current situation (i.e
nitpicking/bikeshedding on the ML) is not a good use of your time :).

I would be happy to put some effort to help getting the coding style
right, however I believe focusing on an overall coding style would 
value

everyone's time better than a rule by rule discussion.


If I was promised (reasonable) feedback, I could take what I
have and try to add at least a few more things based on what I find
myself commenting on more frequently. But really I'd prefer it to
be done the other way around - for people to look at the patches
already sent, and for me to only subsequently send more. After all,
if already those adjustments are controversial, I don't think we
could settle on others.
While I understand this requires another investment from your part, 
I am
afraid it is going to be painful for someone else to go through all 
the

existing coding style bikeshedding and infer your unwritten rules.

It might be more beneficial for that person to pursue the work done by
Tamas and Viktor in the past (see my previous e-mail). This may 
mean to

adopt an existing coding style (BSD) and then tweak it.


Thanks Julien for restarting this discussion. IMHO agreeing on a set
of style rules ahead and then applying universally all at once is not
going to be productive since we are so all over the place. Instead, I
would recommend we start piece-by-piece. We introduce a baseline style
checker, then maintainers can decide when and if they want to move
their code-base to be under the automated style checker. That way we
have a baseline and each maintainer can decide on their own term when
they want to have their files be also style checked and in what form.
The upside of this route I think is pretty clear: we can have at least
partial automation even while we figure out what to do with some of
the more problematic files and quirks that are in our code-base. I
would highly prefer this route since I would immediately bring all
files I maintain over to the automated checker just so I never ever
have to deal with this again manually. What style is in use to me
really doesn't matter, BSD was very close with some minor tweaks, or
even what we use to check the style just as long as we have
_something_.


Wouldn't it make more sense to have a patch checker instead and accept
only patches which change code according to the style guide? This
wouldn't require to change complete files at a time.


So what are you going to do if a new contributor is unfortunate enough 
to modify code that doesn't pass the coding style checker? Are we going 
to impose him/her to fix the coding style before submitting his patch?




In theory, yes. But in practice this would require that we can agree
on a style that applies to all patches that touch any file within Xen.
We can't seem to do that because there are too many exceptions and
corner-cases and personal-preferences of maintainers that apply only
to a subset of the codebase. So AFAICT what you propose doesn't seem
to be a viable way to start.


I think long ago we already agreed to have a control file which tells a
style checker which style to apply (if any). As a start we could have a
patch checker checking only the commit message (has it a Signed-off-by:
etc.). The next step would be to add the control file, and the framework
to split the patch into the changed file hunks and passing each hunk to
the correct checking script (might be doing nothing in the beginning).
And then we could add some logic to the single checkers.


Yes the framework can be written down now (patches are welcomed). But 
this doesn't resolve the underlying problem. Aside files imported from 
Linux, what is the coding style of Xen?


The best way to describe it at the moment is a collection of unwritten 
rules that can differ even between maintainers of the same component. I 
don't really see how we could write a style checker based on this.


The best way to reduce the burden on reviewer and bikeshedding is by 
formalizing what our coding style. With that, we can write a style 
checker effectively and all sort of other tools.


It would prefer to have a common one but we could 

[xen-4.12-testing test] 150072: tolerable FAIL - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qcow217 guest-localmigrate/x10   fail  like 150041
 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-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 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-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-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 14 saverestore-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-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-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-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-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 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-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  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-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-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-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-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-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass

version targeted for testing:
 xen  c26841f0aad0af887a6b3658d71959c4946e480d
baseline version:
 xen  e43fc14ec58329813af876ed3b30899a04d65a08

Last test of basis   150041  2020-05-05 16:06:33 Z2 days
Testing same since   150072  2020-05-07 13:06:22 Z1 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Ashok Raj 
  Borislav Petkov 
  Hongyan Xia 
  Jan Beulich 
  Juergen Gross 
  Roger Pau Monné 
  Thomas Gleixner 
  Varad Gautam 

jobs:
 build-amd64-xsm  

[PATCH v8 09/12] xen: add runtime parameter access support to hypfs

[Juergen Gross]

Add support to read and modify values of hypervisor runtime parameters
via the hypervisor file system.

As runtime parameters can be modified via a sysctl, too, this path has
to take the hypfs rw_lock as writer.

For custom runtime parameters the connection between the parameter
value and the file system is done via an init function which will set
the initial value (if needed) and the leaf properties.

Signed-off-by: Juergen Gross 
---
V3:
- complete rework
- support custom parameters, too
- support parameter writing

V6:
- rewording in docs/misc/hypfs-paths.pandoc (Jan Beulich)
- use memchr() (Jan Beulich)
- use strlcat() (Jan Beulich)
- rework to use a custom parameter init function instead of a reference
  to a content variable, allowing to drop default strings
- style correction (Jan Beulich)
- dropping param_append_str() in favor of a custom function at its only
  use site

V7:
- fine tune some parameter initializations (Jan Beulich)
- call custom_runtime_set_var() after updating the value
- modify alignment in Arm linker script to 4 (Jan Beulich)

V8:
- modify alignment in Arm linker script to 8 (Julien Grall)
- fix ept runtime parameter reporting (Jan Beulich)
- rebase to support CONFIG_HYPFS

Signed-off-by: Juergen Gross 
---
 docs/misc/hypfs-paths.pandoc |   9 +++
 xen/arch/arm/xen.lds.S   |   7 ++
 xen/arch/x86/hvm/vmx/vmcs.c  |  29 +++-
 xen/arch/x86/pv/domain.c |  24 ++-
 xen/arch/x86/xen.lds.S   |   7 ++
 xen/common/grant_table.c |  62 ++
 xen/common/hypfs.c   |  41 
 xen/common/kernel.c  |  27 +++-
 xen/drivers/char/console.c   |  77 --
 xen/include/xen/hypfs.h  |   7 ++
 xen/include/xen/param.h  | 124 ++-
 11 files changed, 390 insertions(+), 24 deletions(-)

diff --git a/docs/misc/hypfs-paths.pandoc b/docs/misc/hypfs-paths.pandoc
index 9a76bc383b..a111c6f25c 100644
--- a/docs/misc/hypfs-paths.pandoc
+++ b/docs/misc/hypfs-paths.pandoc
@@ -154,3 +154,12 @@ The major version of Xen.
  /buildinfo/version/minor = INTEGER
 
 The minor version of Xen.
+
+ /params/
+
+A directory of runtime parameters.
+
+ /params/*
+
+The individual parameters. The description of the different parameters can be
+found in `docs/misc/xen-command-line.pandoc`.
diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index a497f6a48d..0a6efe96cf 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -89,6 +89,13 @@ SECTIONS
__start_schedulers_array = .;
*(.data.schedulers)
__end_schedulers_array = .;
+
+#ifdef CONFIG_HYPFS
+   . = ALIGN(8);
+   __paramhypfs_start = .;
+   *(.data.paramhypfs)
+   __paramhypfs_end = .;
+#endif
*(.data.rel)
*(.data.rel.*)
CONSTRUCTORS
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 221af9737a..1746385d13 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -71,6 +71,27 @@ static bool __read_mostly opt_ept_pml = true;
 static s8 __read_mostly opt_ept_ad = -1;
 int8_t __read_mostly opt_ept_exec_sp = -1;
 
+#ifdef CONFIG_HYPFS
+static char opt_ept_setting[10];
+
+static void update_ept_param(void)
+{
+if ( opt_ept_exec_sp >= 0 )
+snprintf(opt_ept_setting, sizeof(opt_ept_setting), "exec-sp=%d",
+ opt_ept_exec_sp);
+}
+
+static void __init init_ept_param(struct param_hypfs *par)
+{
+update_ept_param();
+custom_runtime_set_var(par, opt_ept_setting);
+}
+#else
+static void update_ept_param(void)
+{
+}
+#endif
+
 static int __init parse_ept_param(const char *s)
 {
 const char *ss;
@@ -97,6 +118,9 @@ static int __init parse_ept_param(const char *s)
 }
 custom_param("ept", parse_ept_param);
 
+static int parse_ept_param_runtime(const char *s);
+custom_runtime_only_param("ept", parse_ept_param_runtime, init_ept_param);
+
 static int parse_ept_param_runtime(const char *s)
 {
 struct domain *d;
@@ -115,6 +139,10 @@ static int parse_ept_param_runtime(const char *s)
 
 opt_ept_exec_sp = val;
 
+update_ept_param();
+custom_runtime_set_var(param_2_parfs(parse_ept_param_runtime),
+   opt_ept_setting);
+
 rcu_read_lock(_read_lock);
 for_each_domain ( d )
 {
@@ -144,7 +172,6 @@ static int parse_ept_param_runtime(const char *s)
 
 return 0;
 }
-custom_runtime_only_param("ept", parse_ept_param_runtime);
 
 /* Dynamic (run-time adjusted) execution control flags. */
 u32 vmx_pin_based_exec_control __read_mostly;
diff --git a/xen/arch/x86/pv/domain.c b/xen/arch/x86/pv/domain.c
index 0a4a5bd001..6bfce9a76a 100644
--- a/xen/arch/x86/pv/domain.c
+++ b/xen/arch/x86/pv/domain.c
@@ -52,9 +52,27 @@ static __read_mostly enum {
 PCID_OFF,
 PCID_ALL,
 PCID_XPTI,
-PCID_NOXPTI
+PCID_NOXPTI,
+PCID_END
 } opt_pcid = PCID_XPTI;
 
+#ifdef CONFIG_HYPFS
+static const char opt_pcid_2_string[PCID_END][7] = {
+[PCID_OFF] = "off",
+

[PATCH v8 00/12] Add hypervisor sysfs-like support

[Juergen Gross]

On the 2019 Xen developer summit there was agreement that the Xen
hypervisor should gain support for a hierarchical name-value store
similar to the Linux kernel's sysfs.

This is a first implementation of that idea adding the basic
functionality to hypervisor and tools side. The interface to any
user program making use of that "xen-hypfs" is a new library
"libxenhypfs" with a stable interface.

The series adds read-only nodes with buildinfo data and writable
nodes with runtime parameters. xl is switched to use the new file
system for modifying the runtime parameters and the old sysctl
interface for that purpose is dropped.

Changes in V8:
- addressed review comments
- added CONFIG_HYPFS config option

Changes in V7:
- old patch 1 already applied
- add new patch 1 (carved out and modified from patch 9)
- addressed review comments
- modified public interface to have a max write size instead of a
  writable flag only

Changes in V6:
- added new patches 1, 10, 11, 12
- addressed review comments
- modified interface for creating nodes for runtime parameters

Changes in V5:
- switched to xsm for privilege check

Changes in V4:
- former patch 2 removed as already committed
- addressed review comments

Changes in V3:
- major rework, especially by supporting binary contents of entries
- added several new patches (1, 2, 7)
- full support of all runtime parameters
- support of writing entries (especially runtime parameters)

Changes in V2:
- all comments to V1 addressed
- added man-page for xenhypfs tool
- added runtime parameter read access for string parameters

Changes in V1:
- renamed xenfs ->xenhypfs
- added writable entries support at the interface level and in the
  xenhypfs tool
- added runtime parameter read access (integer type only for now)
- added docs/misc/hypfs-paths.pandoc for path descriptions

Juergen Gross (12):
  xen/vmx: let opt_ept_ad always reflect the current setting
  xen: add a generic way to include binary files as variables
  docs: add feature document for Xen hypervisor sysfs-like support
  xen: add basic hypervisor filesystem support
  libs: add libxenhypfs
  tools: add xenfs tool
  xen: provide version information in hypfs
  xen: add /buildinfo/config entry to hypervisor filesystem
  xen: add runtime parameter access support to hypfs
  tools/libxl: use libxenhypfs for setting xen runtime parameters
  tools/libxc: remove xc_set_parameters()
  xen: remove XEN_SYSCTL_set_parameter support

 .gitignore  |   6 +
 docs/features/hypervisorfs.pandoc   |  92 +
 docs/man/xenhypfs.1.pod |  61 
 docs/misc/hypfs-paths.pandoc| 165 +
 tools/Rules.mk  |   8 +-
 tools/flask/policy/modules/dom0.te  |   4 +-
 tools/libs/Makefile |   1 +
 tools/libs/hypfs/Makefile   |  16 +
 tools/libs/hypfs/core.c | 536 
 tools/libs/hypfs/include/xenhypfs.h |  90 +
 tools/libs/hypfs/libxenhypfs.map|  10 +
 tools/libs/hypfs/xenhypfs.pc.in |  10 +
 tools/libxc/include/xenctrl.h   |   1 -
 tools/libxc/xc_misc.c   |  21 --
 tools/libxl/Makefile|   3 +-
 tools/libxl/libxl.c |  53 ++-
 tools/libxl/libxl_internal.h|   1 +
 tools/libxl/xenlight.pc.in  |   2 +-
 tools/misc/Makefile |   6 +
 tools/misc/xenhypfs.c   | 192 ++
 tools/xl/xl_misc.c  |   1 -
 xen/arch/arm/traps.c|   3 +
 xen/arch/arm/xen.lds.S  |  12 +-
 xen/arch/x86/hvm/hypercall.c|   3 +
 xen/arch/x86/hvm/vmx/vmcs.c |  47 ++-
 xen/arch/x86/hvm/vmx/vmx.c  |   4 +-
 xen/arch/x86/hypercall.c|   3 +
 xen/arch/x86/pv/domain.c|  24 +-
 xen/arch/x86/pv/hypercall.c |   3 +
 xen/arch/x86/xen.lds.S  |  12 +-
 xen/common/Kconfig  |  22 ++
 xen/common/Makefile |  13 +
 xen/common/grant_table.c|  62 +++-
 xen/common/hypfs.c  | 385 
 xen/common/kernel.c |  82 -
 xen/common/sysctl.c |  36 --
 xen/drivers/char/console.c  |  77 +++-
 xen/include/Makefile|   1 +
 xen/include/asm-x86/hvm/vmx/vmcs.h  |   3 +-
 xen/include/public/hypfs.h  | 127 +++
 xen/include/public/sysctl.h |  19 +-
 xen/include/public/xen.h|   1 +
 xen/include/xen/hypercall.h |  10 +
 xen/include/xen/hypfs.h | 124 +++
 xen/include/xen/kernel.h|   3 +
 xen/include/xen/lib.h   |   1 -
 xen/include/xen/param.h | 123 +--
 xen/include/xlat.lst|   2 +
 xen/include/xsm/dummy.h |   6 +
 xen/include/xsm/xsm.h   |   6 +
 xen/tools/binfile   |  41 +++
 xen/xsm/dummy.c |   1 +
 xen/xsm/flask/Makefile  |   5 +-
 xen/xsm/flask/flask-policy.S|  16 

[PATCH v8 12/12] xen: remove XEN_SYSCTL_set_parameter support

[Juergen Gross]

The functionality of XEN_SYSCTL_set_parameter is available via hypfs
now, so it can be removed.

This allows to remove the kernel_param structure for runtime parameters
by putting the now only used structure element into the hypfs node
structure of the runtime parameters.

Signed-off-by: Juergen Gross 
---
V6:
- new patch

V7:
- only comment out definition of XEN_SYSCTL_set_parameter (Jan Beulich)

V8:
- rebase to use CONFIG_HYPFS

Signed-off-by: Juergen Gross 
---
 tools/flask/policy/modules/dom0.te  |  2 +-
 xen/arch/arm/xen.lds.S  |  5 --
 xen/arch/x86/hvm/vmx/vmcs.c | 38 ++---
 xen/arch/x86/xen.lds.S  |  5 --
 xen/common/hypfs.c  | 12 +---
 xen/common/kernel.c | 11 
 xen/common/sysctl.c | 36 
 xen/include/public/sysctl.h | 19 +--
 xen/include/xen/hypfs.h |  5 --
 xen/include/xen/lib.h   |  1 -
 xen/include/xen/param.h | 87 +
 xen/xsm/flask/hooks.c   |  3 -
 xen/xsm/flask/policy/access_vectors |  2 -
 13 files changed, 34 insertions(+), 192 deletions(-)

diff --git a/tools/flask/policy/modules/dom0.te 
b/tools/flask/policy/modules/dom0.te
index 20925e38a2..0a63ce15b6 100644
--- a/tools/flask/policy/modules/dom0.te
+++ b/tools/flask/policy/modules/dom0.te
@@ -16,7 +16,7 @@ allow dom0_t xen_t:xen {
 allow dom0_t xen_t:xen2 {
resource_op psr_cmt_op psr_alloc pmu_ctrl get_symbol
get_cpu_levelling_caps get_cpu_featureset livepatch_op
-   coverage_op set_parameter
+   coverage_op
 };
 
 # Allow dom0 to use all XENVER_ subops that have checks.
diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 0a6efe96cf..a795497ac8 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -54,11 +54,6 @@ SECTIONS
*(.data.rel.ro)
*(.data.rel.ro.*)
 
-   . = ALIGN(POINTER_ALIGN);
-   __param_start = .;
-   *(.data.param)
-   __param_end = .;
-
__proc_info_start = .;
*(.proc.info)
__proc_info_end = .;
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 1746385d13..ca94c2bedc 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -71,27 +71,6 @@ static bool __read_mostly opt_ept_pml = true;
 static s8 __read_mostly opt_ept_ad = -1;
 int8_t __read_mostly opt_ept_exec_sp = -1;
 
-#ifdef CONFIG_HYPFS
-static char opt_ept_setting[10];
-
-static void update_ept_param(void)
-{
-if ( opt_ept_exec_sp >= 0 )
-snprintf(opt_ept_setting, sizeof(opt_ept_setting), "exec-sp=%d",
- opt_ept_exec_sp);
-}
-
-static void __init init_ept_param(struct param_hypfs *par)
-{
-update_ept_param();
-custom_runtime_set_var(par, opt_ept_setting);
-}
-#else
-static void update_ept_param(void)
-{
-}
-#endif
-
 static int __init parse_ept_param(const char *s)
 {
 const char *ss;
@@ -118,6 +97,22 @@ static int __init parse_ept_param(const char *s)
 }
 custom_param("ept", parse_ept_param);
 
+#ifdef CONFIG_HYPFS
+static char opt_ept_setting[10];
+
+static void update_ept_param(void)
+{
+if ( opt_ept_exec_sp >= 0 )
+snprintf(opt_ept_setting, sizeof(opt_ept_setting), "exec-sp=%d",
+ opt_ept_exec_sp);
+}
+
+static void __init init_ept_param(struct param_hypfs *par)
+{
+update_ept_param();
+custom_runtime_set_var(par, opt_ept_setting);
+}
+
 static int parse_ept_param_runtime(const char *s);
 custom_runtime_only_param("ept", parse_ept_param_runtime, init_ept_param);
 
@@ -172,6 +167,7 @@ static int parse_ept_param_runtime(const char *s)
 
 return 0;
 }
+#endif
 
 /* Dynamic (run-time adjusted) execution control flags. */
 u32 vmx_pin_based_exec_control __read_mostly;
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 3ed020e26b..0273f79152 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -128,11 +128,6 @@ SECTIONS
*(.ex_table.pre)
__stop___pre_ex_table = .;
 
-   . = ALIGN(POINTER_ALIGN);
-   __param_start = .;
-   *(.data.param)
-   __param_end = .;
-
 #if defined(CONFIG_HAS_VPCI) && defined(CONFIG_LATE_HWDOM)
. = ALIGN(POINTER_ALIGN);
__start_vpci_array = .;
diff --git a/xen/common/hypfs.c b/xen/common/hypfs.c
index e4de0a9eef..434b6c1308 100644
--- a/xen/common/hypfs.c
+++ b/xen/common/hypfs.c
@@ -302,7 +302,7 @@ int hypfs_write_custom(struct hypfs_entry_leaf *leaf,
 goto out;
 
 p = container_of(leaf, struct param_hypfs, hypfs);
-ret = p->param->par.func(buf);
+ret = p->func(buf);
 
 if ( !ret )
 leaf->e.size = ulen;
@@ -383,13 +383,3 @@ long do_hypfs_op(unsigned int cmd,
 
 return ret;
 }
-
-void hypfs_write_lock(void)
-{
-write_lock(_lock);
-}
-
-void hypfs_write_unlock(void)
-{
-write_unlock(_lock);
-}
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 3b8b0d8ca5..bb37c7c9bb 100644
--- a/xen/common/kernel.c

[PATCH v8 03/12] docs: add feature document for Xen hypervisor sysfs-like support

[Juergen Gross]

On the 2019 Xen developer summit there was agreement that the Xen
hypervisor should gain support for a hierarchical name-value store
similar to the Linux kernel's sysfs.

In the beginning there should only be basic support: entries can be
added from the hypervisor itself only, there is a simple hypercall
interface to read the data.

Add a feature document for setting the base of a discussion regarding
the desired functionality and the entries to add.

Signed-off-by: Juergen Gross 
Acked-by: Julien Grall 
---
V1:
- remove the "--" prefixes of the sub-commands of the user tool
  (Jan Beulich)
- rename xenfs to xenhypfs (Jan Beulich)
- add "tree" and "write" options to user tool

V2:
- move example tree to the paths description (Ian Jackson)
- specify allowed characters for keys and values (Ian Jackson)

V3:
- correct introduction (writable entries)

V4:
- add list specification
- add entry example (Julien Grall)
- correct date and Xen version (Julien Grall)
- add ARM64 as possible architecture (Julien Grall)
- add version description to the feature doc (Jan Beulich)

V8:
- clarify syntax used in hypfs-paths.pandoc (George Dunlap)

Signed-off-by: Juergen Gross 
---
 docs/features/hypervisorfs.pandoc |  92 +
 docs/misc/hypfs-paths.pandoc  | 107 ++
 2 files changed, 199 insertions(+)
 create mode 100644 docs/features/hypervisorfs.pandoc
 create mode 100644 docs/misc/hypfs-paths.pandoc

diff --git a/docs/features/hypervisorfs.pandoc 
b/docs/features/hypervisorfs.pandoc
new file mode 100644
index 00..a0a0ead057
--- /dev/null
+++ b/docs/features/hypervisorfs.pandoc
@@ -0,0 +1,92 @@
+% Hypervisor FS
+% Revision 1
+
+\clearpage
+
+# Basics
+ -
+ Status: **Supported**
+
+  Architectures: all
+
+ Components: Hypervisor, toolstack
+ -
+
+# Overview
+
+The Hypervisor FS is a hierarchical name-value store for reporting
+information to guests, especially dom0. It is similar to the Linux
+kernel's sysfs. Entries and directories are created by the hypervisor,
+while the toolstack is able to use a hypercall to query the entry
+values or (if allowed by the hypervisor) to modify them.
+
+# User details
+
+With:
+
+xenhypfs ls 
+
+the user can list the entries of a specific path of the FS. Using:
+
+xenhypfs cat 
+
+the content of an entry can be retrieved. Using:
+
+xenhypfs write  
+
+a writable entry can be modified. With:
+
+xenhypfs tree
+
+the complete Hypervisor FS entry tree can be printed.
+
+The FS paths are documented in `docs/misc/hypfs-paths.pandoc`.
+
+# Technical details
+
+Access to the hypervisor filesystem is done via the stable new hypercall
+__HYPERVISOR_filesystem_op. This hypercall supports a sub-command
+XEN_HYPFS_OP_get_version which will return the highest version of the
+interface supported by the hypervisor. Additions to the interface need
+to bump the interface version. The hypervisor is required to support the
+previous interface versions, too (this implies that additions will always
+require new sub-commands in order to allow the hypervisor to decide which
+version of the interface to use).
+
+* hypercall interface specification
+* `xen/include/public/hypfs.h`
+* hypervisor internal files
+* `xen/include/xen/hypfs.h`
+* `xen/common/hypfs.c`
+* `libxenhypfs`
+* `tools/libs/libxenhypfs/*`
+* `xenhypfs`
+* `tools/misc/xenhypfs.c`
+* path documentation
+* `docs/misc/hypfs-paths.pandoc`
+
+# Testing
+
+Any new parameters or hardware mitigations should be verified to show up
+correctly in the filesystem.
+
+# Areas for improvement
+
+* More detailed access rights
+* Entries per domain and/or per cpupool
+
+# Known issues
+
+* None
+
+# References
+
+* None
+
+# History
+
+
+Date   Revision Version  Notes
+--   ---
+2020-01-23 1Xen 4.14 Document written
+--   ---
diff --git a/docs/misc/hypfs-paths.pandoc b/docs/misc/hypfs-paths.pandoc
new file mode 100644
index 00..39539fa1b5
--- /dev/null
+++ b/docs/misc/hypfs-paths.pandoc
@@ -0,0 +1,107 @@
+# Xenhypfs Paths
+
+This document attempts to define all the paths which are available
+in the Xen hypervisor file system (hypfs).
+
+The hypervisor file system can be accessed via the xenhypfs tool.
+
+## Notation
+
+The hypervisor file system is similar to the Linux kernel's sysfs.
+In this document directories are always specified with a trailing "/".
+
+The following notation conventions apply:
+
+DIRECTORY/
+
+PATH = VALUES [TAGS]
+
+The first syntax defines a directory. It normally contains related
+entries and the general scope of the directory is described.
+
+The second syntax defines a file entry containing values which are
+either set by 

[PATCH v8 02/12] xen: add a generic way to include binary files as variables

[Juergen Gross]

Add a new script xen/tools/binfile for including a binary file at build
time being usable via a pointer and a size variable in the hypervisor.

Make use of that generic tool in xsm.

Signed-off-by: Juergen Gross 
Reviewed-by: Jan Beulich 
Reviewed-by: Wei Liu 
---
V3:
- new patch

V4:
- add alignment parameter (Jan Beulich)
- use .Lend instead of . (Jan Beulich)

Signed-off-by: Juergen Gross 
---
 .gitignore   |  1 +
 xen/tools/binfile| 41 
 xen/xsm/flask/Makefile   |  5 -
 xen/xsm/flask/flask-policy.S | 16 --
 4 files changed, 46 insertions(+), 17 deletions(-)
 create mode 100755 xen/tools/binfile
 delete mode 100644 xen/xsm/flask/flask-policy.S

diff --git a/.gitignore b/.gitignore
index 9c8a31f896..4059829f6e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -314,6 +314,7 @@ xen/test/livepatch/*.livepatch
 xen/tools/kconfig/.tmp_gtkcheck
 xen/tools/kconfig/.tmp_qtcheck
 xen/tools/symbols
+xen/xsm/flask/flask-policy.S
 xen/xsm/flask/include/av_perm_to_string.h
 xen/xsm/flask/include/av_permissions.h
 xen/xsm/flask/include/class_to_string.h
diff --git a/xen/tools/binfile b/xen/tools/binfile
new file mode 100755
index 00..7bb35a5178
--- /dev/null
+++ b/xen/tools/binfile
@@ -0,0 +1,41 @@
+#!/bin/sh
+# usage: binfile [-i] [-a ]   
+# -a   align data at 2^ boundary (default: byte alignment)
+# -i  add to .init.rodata (default: .rodata) section
+
+section=""
+align=0
+
+OPTIND=1
+while getopts "ia:" opt; do
+case "$opt" in
+i)
+section=".init"
+;;
+a)
+align=$OPTARG
+;;
+esac
+done
+
+target=$1
+binsource=$2
+varname=$3
+
+cat <$target
+#include 
+
+.section $section.rodata, "a", %progbits
+
+.p2align $align
+.global $varname
+$varname:
+.incbin "$binsource"
+.Lend:
+
+.type $varname, %object
+.size $varname, .Lend - $varname
+
+.global ${varname}_size
+ASM_INT(${varname}_size, .Lend - $varname)
+EOF
diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
index eebfceecc5..d8486fc7e4 100644
--- a/xen/xsm/flask/Makefile
+++ b/xen/xsm/flask/Makefile
@@ -39,6 +39,9 @@ $(subst include/,%/,$(AV_H_FILES)): $(AV_H_DEPEND) 
$(mkaccess) FORCE
 obj-bin-$(CONFIG_XSM_FLASK_POLICY) += flask-policy.o
 flask-policy.o: policy.bin
 
+flask-policy.S: $(XEN_ROOT)/xen/tools/binfile
+   $(XEN_ROOT)/xen/tools/binfile -i $@ policy.bin xsm_flask_init_policy
+
 FLASK_BUILD_DIR := $(CURDIR)
 POLICY_SRC := $(FLASK_BUILD_DIR)/xenpolicy-$(XEN_FULLVERSION)
 
@@ -48,4 +51,4 @@ policy.bin: FORCE
 
 .PHONY: clean
 clean::
-   rm -f $(ALL_H_FILES) *.o $(DEPS_RM) policy.* $(POLICY_SRC)
+   rm -f $(ALL_H_FILES) *.o $(DEPS_RM) policy.* $(POLICY_SRC) 
flask-policy.S
diff --git a/xen/xsm/flask/flask-policy.S b/xen/xsm/flask/flask-policy.S
deleted file mode 100644
index d38aa39964..00
--- a/xen/xsm/flask/flask-policy.S
+++ /dev/null
@@ -1,16 +0,0 @@
-#include 
-
-.section .init.rodata, "a", %progbits
-
-/* const unsigned char xsm_flask_init_policy[] __initconst */
-.global xsm_flask_init_policy
-xsm_flask_init_policy:
-.incbin "policy.bin"
-.Lend:
-
-.type xsm_flask_init_policy, %object
-.size xsm_flask_init_policy, . - xsm_flask_init_policy
-
-/* const unsigned int __initconst xsm_flask_init_policy_size */
-.global xsm_flask_init_policy_size
-ASM_INT(xsm_flask_init_policy_size, .Lend - xsm_flask_init_policy)
-- 
2.26.1

[PATCH v8 05/12] libs: add libxenhypfs

[Juergen Gross]

Add the new library libxenhypfs for access to the hypervisor filesystem.

Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
---
V1:
- rename to libxenhypfs
- add xenhypfs_write()

V3:
- major rework due to new hypervisor interface
- add decompression capability

V4:
- add dependency to libz in pkgconfig file (Wei Liu)

V7:
- don't assume hypervisor's sizeof(bool) is same as in user land

V8:
- add some comments regarding the semantics of the lib functions
  (George Dunlap)

Signed-off-by: Juergen Gross 
---
 .gitignore  |   2 +
 tools/Rules.mk  |   6 +
 tools/libs/Makefile |   1 +
 tools/libs/hypfs/Makefile   |  16 +
 tools/libs/hypfs/core.c | 536 
 tools/libs/hypfs/include/xenhypfs.h |  90 +
 tools/libs/hypfs/libxenhypfs.map|  10 +
 tools/libs/hypfs/xenhypfs.pc.in |  10 +
 8 files changed, 671 insertions(+)
 create mode 100644 tools/libs/hypfs/Makefile
 create mode 100644 tools/libs/hypfs/core.c
 create mode 100644 tools/libs/hypfs/include/xenhypfs.h
 create mode 100644 tools/libs/hypfs/libxenhypfs.map
 create mode 100644 tools/libs/hypfs/xenhypfs.pc.in

diff --git a/.gitignore b/.gitignore
index 4059829f6e..41b1d4c8a1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -110,6 +110,8 @@ tools/libs/evtchn/headers.chk
 tools/libs/evtchn/xenevtchn.pc
 tools/libs/gnttab/headers.chk
 tools/libs/gnttab/xengnttab.pc
+tools/libs/hypfs/headers.chk
+tools/libs/hypfs/xenhypfs.pc
 tools/libs/call/headers.chk
 tools/libs/call/xencall.pc
 tools/libs/foreignmemory/headers.chk
diff --git a/tools/Rules.mk b/tools/Rules.mk
index 5b8cf748ad..ad6073fcad 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -19,6 +19,7 @@ XEN_LIBXENGNTTAB   = $(XEN_ROOT)/tools/libs/gnttab
 XEN_LIBXENCALL = $(XEN_ROOT)/tools/libs/call
 XEN_LIBXENFOREIGNMEMORY = $(XEN_ROOT)/tools/libs/foreignmemory
 XEN_LIBXENDEVICEMODEL = $(XEN_ROOT)/tools/libs/devicemodel
+XEN_LIBXENHYPFS= $(XEN_ROOT)/tools/libs/hypfs
 XEN_LIBXC  = $(XEN_ROOT)/tools/libxc
 XEN_XENLIGHT   = $(XEN_ROOT)/tools/libxl
 # Currently libxlutil lives in the same directory as libxenlight
@@ -132,6 +133,11 @@ SHDEPS_libxendevicemodel = $(SHLIB_libxentoollog) 
$(SHLIB_libxentoolcore) $(SHLI
 LDLIBS_libxendevicemodel = $(SHDEPS_libxendevicemodel) 
$(XEN_LIBXENDEVICEMODEL)/libxendevicemodel$(libextension)
 SHLIB_libxendevicemodel  = $(SHDEPS_libxendevicemodel) 
-Wl,-rpath-link=$(XEN_LIBXENDEVICEMODEL)
 
+CFLAGS_libxenhypfs = -I$(XEN_LIBXENHYPFS)/include $(CFLAGS_xeninclude)
+SHDEPS_libxenhypfs = $(SHLIB_libxentoollog) $(SHLIB_libxentoolcore) 
$(SHLIB_xencall)
+LDLIBS_libxenhypfs = $(SHDEPS_libxenhypfs) 
$(XEN_LIBXENHYPFS)/libxenhypfs$(libextension)
+SHLIB_libxenhypfs  = $(SHDEPS_libxenhypfs) -Wl,-rpath-link=$(XEN_LIBXENHYPFS)
+
 # code which compiles against libxenctrl get __XEN_TOOLS__ and
 # therefore sees the unstable hypercall interfaces.
 CFLAGS_libxenctrl = -I$(XEN_LIBXC)/include $(CFLAGS_libxentoollog) 
$(CFLAGS_libxenforeignmemory) $(CFLAGS_libxendevicemodel) $(CFLAGS_xeninclude) 
-D__XEN_TOOLS__
diff --git a/tools/libs/Makefile b/tools/libs/Makefile
index 88901e7341..69cdfb5975 100644
--- a/tools/libs/Makefile
+++ b/tools/libs/Makefile
@@ -9,6 +9,7 @@ SUBDIRS-y += gnttab
 SUBDIRS-y += call
 SUBDIRS-y += foreignmemory
 SUBDIRS-y += devicemodel
+SUBDIRS-y += hypfs
 
 ifeq ($(CONFIG_RUMP),y)
 SUBDIRS-y := toolcore
diff --git a/tools/libs/hypfs/Makefile b/tools/libs/hypfs/Makefile
new file mode 100644
index 00..06dd449929
--- /dev/null
+++ b/tools/libs/hypfs/Makefile
@@ -0,0 +1,16 @@
+XEN_ROOT = $(CURDIR)/../../..
+include $(XEN_ROOT)/tools/Rules.mk
+
+MAJOR= 1
+MINOR= 0
+LIBNAME  := hypfs
+USELIBS  := toollog toolcore call
+
+APPEND_LDFLAGS += -lz
+
+SRCS-y += core.c
+
+include ../libs.mk
+
+$(PKG_CONFIG_LOCAL): PKG_CONFIG_INCDIR = $(XEN_LIBXENHYPFS)/include
+$(PKG_CONFIG_LOCAL): PKG_CONFIG_CFLAGS_LOCAL = $(CFLAGS_xeninclude)
diff --git a/tools/libs/hypfs/core.c b/tools/libs/hypfs/core.c
new file mode 100644
index 00..befdc66e96
--- /dev/null
+++ b/tools/libs/hypfs/core.c
@@ -0,0 +1,536 @@
+/*
+ * Copyright (c) 2019 SUSE Software Solutions Germany GmbH
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation;
+ * version 2.1 of the License.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; If not, see .
+ */
+
+#define __XEN_TOOLS__ 1
+
+#define _GNU_SOURCE
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 

[PATCH v8 10/12] tools/libxl: use libxenhypfs for setting xen runtime parameters

[Juergen Gross]

Instead of xc_set_parameters() use xenhypfs_write() for setting
parameters of the hypervisor.

Signed-off-by: Juergen Gross 
---
V6:
- new patch
---
 tools/Rules.mk   |  2 +-
 tools/libxl/Makefile |  3 +-
 tools/libxl/libxl.c  | 53 
 tools/libxl/libxl_internal.h |  1 +
 tools/libxl/xenlight.pc.in   |  2 +-
 tools/xl/xl_misc.c   |  1 -
 6 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/tools/Rules.mk b/tools/Rules.mk
index ad6073fcad..883a193f9e 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -178,7 +178,7 @@ CFLAGS += -O2 -fomit-frame-pointer
 endif
 
 CFLAGS_libxenlight = -I$(XEN_XENLIGHT) $(CFLAGS_libxenctrl) 
$(CFLAGS_xeninclude)
-SHDEPS_libxenlight = $(SHLIB_libxenctrl) $(SHLIB_libxenstore)
+SHDEPS_libxenlight = $(SHLIB_libxenctrl) $(SHLIB_libxenstore) 
$(SHLIB_libxenhypfs)
 LDLIBS_libxenlight = $(SHDEPS_libxenlight) 
$(XEN_XENLIGHT)/libxenlight$(libextension)
 SHLIB_libxenlight  = $(SHDEPS_libxenlight) -Wl,-rpath-link=$(XEN_XENLIGHT)
 
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 69fcf21577..a89ebab0b4 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -20,7 +20,7 @@ LIBUUID_LIBS += -luuid
 endif
 
 LIBXL_LIBS =
-LIBXL_LIBS = $(LDLIBS_libxentoollog) $(LDLIBS_libxenevtchn) 
$(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) 
$(LDLIBS_libxentoolcore) $(PTYFUNCS_LIBS) $(LIBUUID_LIBS)
+LIBXL_LIBS = $(LDLIBS_libxentoollog) $(LDLIBS_libxenevtchn) 
$(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenhypfs) 
$(LDLIBS_libxenstore) $(LDLIBS_libxentoolcore) $(PTYFUNCS_LIBS) $(LIBUUID_LIBS)
 ifeq ($(CONFIG_LIBNL),y)
 LIBXL_LIBS += $(LIBNL3_LIBS)
 endif
@@ -33,6 +33,7 @@ CFLAGS_LIBXL += $(CFLAGS_libxentoolcore)
 CFLAGS_LIBXL += $(CFLAGS_libxenevtchn)
 CFLAGS_LIBXL += $(CFLAGS_libxenctrl)
 CFLAGS_LIBXL += $(CFLAGS_libxenguest)
+CFLAGS_LIBXL += $(CFLAGS_libxenhypfs)
 CFLAGS_LIBXL += $(CFLAGS_libxenstore)
 ifeq ($(CONFIG_LIBNL),y)
 CFLAGS_LIBXL += $(LIBNL3_CFLAGS)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index f60fd3e4fd..621acc88f3 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -663,15 +663,56 @@ int libxl_set_parameters(libxl_ctx *ctx, char *params)
 {
 int ret;
 GC_INIT(ctx);
+char *par, *val, *end, *path;
+xenhypfs_handle *hypfs;
 
-ret = xc_set_parameters(ctx->xch, params);
-if (ret < 0) {
-LOGEV(ERROR, ret, "setting parameters");
-GC_FREE;
-return ERROR_FAIL;
+hypfs = xenhypfs_open(ctx->lg, 0);
+if (!hypfs) {
+LOGE(ERROR, "opening Xen hypfs");
+ret = ERROR_FAIL;
+goto out;
 }
+
+while (isblank(*params))
+params++;
+
+for (par = params; *par; par = end) {
+end = strchr(par, ' ');
+if (!end)
+end = par + strlen(par);
+
+val = strchr(par, '=');
+if (val > end)
+val = NULL;
+if (!val && !strncmp(par, "no", 2)) {
+path = libxl__sprintf(gc, "/params/%s", par + 2);
+path[end - par - 2 + 8] = 0;
+val = "no";
+par += 2;
+} else {
+path = libxl__sprintf(gc, "/params/%s", par);
+path[val - par + 8] = 0;
+val = libxl__strndup(gc, val + 1, end - val - 1);
+}
+
+   LOG(DEBUG, "setting node \"%s\" to value \"%s\"", path, val);
+ret = xenhypfs_write(hypfs, path, val);
+if (ret < 0) {
+LOGE(ERROR, "setting parameters");
+ret = ERROR_FAIL;
+goto out;
+}
+
+while (isblank(*end))
+end++;
+}
+
+ret = 0;
+
+out:
+xenhypfs_close(hypfs);
 GC_FREE;
-return 0;
+return ret;
 }
 
 static int fd_set_flags(libxl_ctx *ctx, int fd,
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index e5effd2ad1..b85b771659 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -56,6 +56,7 @@
 #define XC_WANT_COMPAT_MAP_FOREIGN_API
 #include 
 #include 
+#include 
 #include 
 
 #include 
diff --git a/tools/libxl/xenlight.pc.in b/tools/libxl/xenlight.pc.in
index c0f769fd20..6b351ba096 100644
--- a/tools/libxl/xenlight.pc.in
+++ b/tools/libxl/xenlight.pc.in
@@ -9,4 +9,4 @@ Description: The Xenlight library for Xen hypervisor
 Version: @@version@@
 Cflags: -I${includedir}
 Libs: @@libsflag@@${libdir} -lxenlight
-Requires.private: xentoollog,xenevtchn,xencontrol,xenguest,xenstore
+Requires.private: xentoollog,xenevtchn,xencontrol,xenguest,xenstore,xenhypfs
diff --git a/tools/xl/xl_misc.c b/tools/xl/xl_misc.c
index 20ed605f4f..08f0fb6dc9 100644
--- a/tools/xl/xl_misc.c
+++ b/tools/xl/xl_misc.c
@@ -168,7 +168,6 @@ int main_set_parameters(int argc, char **argv)
 
 if (libxl_set_parameters(ctx, params)) {
 fprintf(stderr, "cannot set parameters: %s\n", params);
-fprintf(stderr, "Use \"xl dmesg\" to look for possible reason.\n");
 return EXIT_FAILURE;
 }

[PATCH v8 01/12] xen/vmx: let opt_ept_ad always reflect the current setting

[Juergen Gross]

In case opt_ept_ad has not been set explicitly by the user via command
line or runtime parameter, it is treated as "no" on Avoton cpus.

Change that handling by setting opt_ept_ad to 0 for this cpu type
explicitly if no user value has been set.

By putting this into the (renamed) boot time initialization of vmcs.c
_vmx_cpu_up() can be made static.

Signed-off-by: Juergen Gross 
Reviewed-by: Jan Beulich 
---
 xen/arch/x86/hvm/vmx/vmcs.c| 22 +++---
 xen/arch/x86/hvm/vmx/vmx.c |  4 +---
 xen/include/asm-x86/hvm/vmx/vmcs.h |  3 +--
 3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 4c23645454..221af9737a 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -315,10 +315,6 @@ static int vmx_init_vmcs_config(void)
 
 if ( !opt_ept_ad )
 _vmx_ept_vpid_cap &= ~VMX_EPT_AD_BIT;
-else if ( /* Work around Erratum AVR41 on Avoton processors. */
-  boot_cpu_data.x86 == 6 && boot_cpu_data.x86_model == 0x4d &&
-  opt_ept_ad < 0 )
-_vmx_ept_vpid_cap &= ~VMX_EPT_AD_BIT;
 
 /*
  * Additional sanity checking before using EPT:
@@ -652,7 +648,7 @@ void vmx_cpu_dead(unsigned int cpu)
 vmx_pi_desc_fixup(cpu);
 }
 
-int _vmx_cpu_up(bool bsp)
+static int _vmx_cpu_up(bool bsp)
 {
 u32 eax, edx;
 int rc, bios_locked, cpu = smp_processor_id();
@@ -2108,9 +2104,21 @@ static void vmcs_dump(unsigned char ch)
 printk("**\n");
 }
 
-void __init setup_vmcs_dump(void)
+int __init vmx_vmcs_init(void)
 {
-register_keyhandler('v', vmcs_dump, "dump VT-x VMCSs", 1);
+int ret;
+
+if ( opt_ept_ad < 0 )
+/* Work around Erratum AVR41 on Avoton processors. */
+opt_ept_ad = !(boot_cpu_data.x86 == 6 &&
+   boot_cpu_data.x86_model == 0x4d);
+
+ret = _vmx_cpu_up(true);
+
+if ( !ret )
+register_keyhandler('v', vmcs_dump, "dump VT-x VMCSs", 1);
+
+return ret;
 }
 
 static void __init __maybe_unused build_assertions(void)
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 6efa80e422..11a4dd94cf 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2482,7 +2482,7 @@ const struct hvm_function_table * __init start_vmx(void)
 {
 set_in_cr4(X86_CR4_VMXE);
 
-if ( _vmx_cpu_up(true) )
+if ( vmx_vmcs_init() )
 {
 printk("VMX: failed to initialise.\n");
 return NULL;
@@ -2553,8 +2553,6 @@ const struct hvm_function_table * __init start_vmx(void)
 vmx_function_table.get_guest_bndcfgs = vmx_get_guest_bndcfgs;
 }
 
-setup_vmcs_dump();
-
 lbr_tsx_fixup_check();
 bdf93_fixup_check();
 
diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h 
b/xen/include/asm-x86/hvm/vmx/vmcs.h
index 95c1dea7b8..906810592f 100644
--- a/xen/include/asm-x86/hvm/vmx/vmcs.h
+++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
@@ -21,11 +21,10 @@
 #include 
 
 extern void vmcs_dump_vcpu(struct vcpu *v);
-extern void setup_vmcs_dump(void);
+extern int vmx_vmcs_init(void);
 extern int  vmx_cpu_up_prepare(unsigned int cpu);
 extern void vmx_cpu_dead(unsigned int cpu);
 extern int  vmx_cpu_up(void);
-extern int  _vmx_cpu_up(bool bsp);
 extern void vmx_cpu_down(void);
 
 struct vmcs_struct {
-- 
2.26.1

[PATCH v8 04/12] xen: add basic hypervisor filesystem support

[Juergen Gross]

Add the infrastructure for the hypervisor filesystem.

This includes the hypercall interface and the base functions for
entry creation, deletion and modification.

In order not to have to repeat the same pattern multiple times in case
adding a new node should BUG_ON() failure, the helpers for adding a
node (hypfs_add_dir() and hypfs_add_leaf()) get a nofault parameter
causing the BUG() in case of a failure.

Signed-off-by: Juergen Gross 
---
V1:
- rename files from filesystem.* to hypfs.*
- add dummy write entry support
- rename hypercall filesystem_op to hypfs_op
- add support for unsigned integer entries

V2:
- test new entry name to be valid

V3:
- major rework, especially by supporting binary contents of entries
- addressed all comments

V4:
- sort #includes alphabetically (Wei Liu)
- add public interface structures to xlat.lst (Jan Beulich)
- let DIRENTRY_SIZE() add 1 for trailing nul byte (Jan Beulich)
- remove hypfs_add_entry() (Jan Beulich)
- len -> ulen (Jan Beulich)
- switch sequence of tests in hypfs_get_entry_rel() (Jan Beulich)
- add const qualifier (Jan Beulich)
- return -ENOBUFS if only direntry but no entry contents are returned
  (Jan Beulich)
- use xmalloc() instead of xzalloc() (Jan Beulich)
- better error handling in hypfs_write_leaf() (Jan Beulich)
- return -EOPNOTSUPP for unknown sub-command (Jan Beulich)
- use plain integers for enum-like constants in public interface
  (Jan Beulich)
- rename XEN_HYPFS_OP_read_contents to XEN_HYPFS_OP_read (Jan Beulich)
- add some comments in include/public/hypfs.h (Jan Beulich)
- use const_char for user parameter path (Jan Beulich)
- add helpers for XEN_HYPFS_TYPE_BOOL and XEN_HYPFS_TYPE_INT entry
  definitions (Jan Beulich)
- make statically defined entries __read_mostly (Jan Beulich)

V5:
- switch to xsm for privilege check

V6:
- use memchr() for testing correct string length (Jan Beulich)
- reject writing to non-string leafs with wrong length (Jan Beulich)
- only support bools of natural size (Julien Grall)
- adjust blank padding in header (Jan Beulich)
- adjust comments in public header (Jan Beulich)
- rename hypfs_string_set() and add comment (Jan Beulich)
- add common HYPFS_INIT helper macro (Jan Beulich)
- really check structures added to xlat.lst (Jan Beulich)
- add missing xsm parts (Jan Beulich)

V7:
- simplify compat check (Jan Beulich)
- add max write size (Jan Beulich)
- better length testing of written string (Jan Beulich)

V8:
- add Kconfig item CONFIG_HYPFS (Jan Beulich)
- init write pointer in HYPFS_*_INIT_WRITABLE() (Jan Beulich)
- expand write ASSERT()s (Jan Beulich)

Signed-off-by: Juergen Gross 
---
 tools/flask/policy/modules/dom0.te  |   2 +-
 xen/arch/arm/traps.c|   3 +
 xen/arch/x86/hvm/hypercall.c|   3 +
 xen/arch/x86/hypercall.c|   3 +
 xen/arch/x86/pv/hypercall.c |   3 +
 xen/common/Kconfig  |  11 +
 xen/common/Makefile |   1 +
 xen/common/hypfs.c  | 354 
 xen/include/Makefile|   1 +
 xen/include/public/hypfs.h  | 127 ++
 xen/include/public/xen.h|   1 +
 xen/include/xen/hypercall.h |  10 +
 xen/include/xen/hypfs.h | 122 ++
 xen/include/xlat.lst|   2 +
 xen/include/xsm/dummy.h |   6 +
 xen/include/xsm/xsm.h   |   6 +
 xen/xsm/dummy.c |   1 +
 xen/xsm/flask/hooks.c   |   6 +
 xen/xsm/flask/policy/access_vectors |   2 +
 19 files changed, 663 insertions(+), 1 deletion(-)
 create mode 100644 xen/common/hypfs.c
 create mode 100644 xen/include/public/hypfs.h
 create mode 100644 xen/include/xen/hypfs.h

diff --git a/tools/flask/policy/modules/dom0.te 
b/tools/flask/policy/modules/dom0.te
index 272f6a4f75..20925e38a2 100644
--- a/tools/flask/policy/modules/dom0.te
+++ b/tools/flask/policy/modules/dom0.te
@@ -11,7 +11,7 @@ allow dom0_t xen_t:xen {
mtrr_del mtrr_read microcode physinfo quirk writeconsole readapic
writeapic privprofile nonprivprofile kexec firmware sleep frequency
getidle debug getcpuinfo heap pm_op mca_op lockprof cpupool_op
-   getscheduler setscheduler
+   getscheduler setscheduler hypfs_op
 };
 allow dom0_t xen_t:xen2 {
resource_op psr_cmt_op psr_alloc pmu_ctrl get_symbol
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 30c4c1830b..8f40d0e0b6 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1381,6 +1381,9 @@ static arm_hypercall_t arm_hypercall_table[] = {
 #ifdef CONFIG_ARGO
 HYPERCALL(argo_op, 5),
 #endif
+#ifdef CONFIG_HYPFS
+HYPERCALL(hypfs_op, 5),
+#endif
 };
 
 #ifndef NDEBUG
diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
index c41c2179c9..b6ccaf4457 100644
--- a/xen/arch/x86/hvm/hypercall.c
+++ b/xen/arch/x86/hvm/hypercall.c
@@ -150,6 +150,9 @@ static const hypercall_table_t hvm_hypercall_table[] = {
 #endif
 HYPERCALL(xenpmu_op),
 

[PATCH v8 11/12] tools/libxc: remove xc_set_parameters()

[Juergen Gross]

There is no user of xc_set_parameters() left, so remove it.

Signed-off-by: Juergen Gross 
---
V6:
- new patch
---
 tools/libxc/include/xenctrl.h |  1 -
 tools/libxc/xc_misc.c | 21 -
 2 files changed, 22 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 0a6ff93229..feb20319b6 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1226,7 +1226,6 @@ int xc_readconsolering(xc_interface *xch,
int clear, int incremental, uint32_t *pindex);
 
 int xc_send_debug_keys(xc_interface *xch, const char *keys);
-int xc_set_parameters(xc_interface *xch, const char *params);
 
 typedef struct xen_sysctl_physinfo xc_physinfo_t;
 typedef struct xen_sysctl_cputopo xc_cputopo_t;
diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
index fe477bf344..3820394413 100644
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -187,27 +187,6 @@ int xc_send_debug_keys(xc_interface *xch, const char *keys)
 return ret;
 }
 
-int xc_set_parameters(xc_interface *xch, const char *params)
-{
-int ret, len = strlen(params);
-DECLARE_SYSCTL;
-DECLARE_HYPERCALL_BOUNCE_IN(params, len);
-
-if ( xc_hypercall_bounce_pre(xch, params) )
-return -1;
-
-sysctl.cmd = XEN_SYSCTL_set_parameter;
-set_xen_guest_handle(sysctl.u.set_parameter.params, params);
-sysctl.u.set_parameter.size = len;
-memset(sysctl.u.set_parameter.pad, 0, sizeof(sysctl.u.set_parameter.pad));
-
-ret = do_sysctl(xch, );
-
-xc_hypercall_bounce_post(xch, params);
-
-return ret;
-}
-
 int xc_physinfo(xc_interface *xch,
 xc_physinfo_t *put_info)
 {
-- 
2.26.1

[PATCH v8 07/12] xen: provide version information in hypfs

[Juergen Gross]

Provide version and compile information in /buildinfo/ node of the
Xen hypervisor file system. As this information is accessible by dom0
only no additional security problem arises.

Signed-off-by: Juergen Gross 
Reviewed-by: Jan Beulich 
---
V3:
- new patch

V4:
- add __read_mostly annotations (Jan Beulich)

Signed-off-by: Juergen Gross 
---
 docs/misc/hypfs-paths.pandoc | 45 ++
 xen/common/kernel.c  | 47 
 2 files changed, 92 insertions(+)

diff --git a/docs/misc/hypfs-paths.pandoc b/docs/misc/hypfs-paths.pandoc
index 39539fa1b5..d730caf394 100644
--- a/docs/misc/hypfs-paths.pandoc
+++ b/docs/misc/hypfs-paths.pandoc
@@ -105,3 +105,48 @@ A populated Xen hypervisor file system might look like the 
following example:
  /
 
 The root of the hypervisor file system.
+
+ /buildinfo/
+
+A directory containing static information generated while building the
+hypervisor.
+
+ /buildinfo/changeset = STRING
+
+Git commit of the hypervisor.
+
+ /buildinfo/compileinfo/
+
+A directory containing information about compilation of Xen.
+
+ /buildinfo/compileinfo/compile_by = STRING
+
+Information who compiled the hypervisor.
+
+ /buildinfo/compileinfo/compile_date = STRING
+
+Date of the hypervisor compilation.
+
+ /buildinfo/compileinfo/compile_domain = STRING
+
+Information about the compile domain.
+
+ /buildinfo/compileinfo/compiler = STRING
+
+The compiler used to build Xen.
+
+ /buildinfo/version/
+
+A directory containing version information of the hypervisor.
+
+ /buildinfo/version/extra = STRING
+
+Extra version information.
+
+ /buildinfo/version/major = INTEGER
+
+The major version of Xen.
+
+ /buildinfo/version/minor = INTEGER
+
+The minor version of Xen.
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 572e3fc07d..db7bd23fcb 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -373,6 +374,52 @@ void __init do_initcalls(void)
 (*call)();
 }
 
+#ifdef CONFIG_HYPFS
+static unsigned int __read_mostly major_version;
+static unsigned int __read_mostly minor_version;
+
+static HYPFS_DIR_INIT(buildinfo, "buildinfo");
+static HYPFS_DIR_INIT(compileinfo, "compileinfo");
+static HYPFS_DIR_INIT(version, "version");
+static HYPFS_UINT_INIT(major, "major", major_version);
+static HYPFS_UINT_INIT(minor, "minor", minor_version);
+static HYPFS_STRING_INIT(changeset, "changeset");
+static HYPFS_STRING_INIT(compiler, "compiler");
+static HYPFS_STRING_INIT(compile_by, "compile_by");
+static HYPFS_STRING_INIT(compile_date, "compile_date");
+static HYPFS_STRING_INIT(compile_domain, "compile_domain");
+static HYPFS_STRING_INIT(extra, "extra");
+
+static int __init buildinfo_init(void)
+{
+hypfs_add_dir(_root, , true);
+
+hypfs_string_set_reference(, xen_changeset());
+hypfs_add_leaf(, , true);
+
+hypfs_add_dir(, , true);
+hypfs_string_set_reference(, xen_compiler());
+hypfs_string_set_reference(_by, xen_compile_by());
+hypfs_string_set_reference(_date, xen_compile_date());
+hypfs_string_set_reference(_domain, xen_compile_domain());
+hypfs_add_leaf(, , true);
+hypfs_add_leaf(, _by, true);
+hypfs_add_leaf(, _date, true);
+hypfs_add_leaf(, _domain, true);
+
+major_version = xen_major_version();
+minor_version = xen_minor_version();
+hypfs_add_dir(, , true);
+hypfs_string_set_reference(, xen_extra_version());
+hypfs_add_leaf(, , true);
+hypfs_add_leaf(, , true);
+hypfs_add_leaf(, , true);
+
+return 0;
+}
+__initcall(buildinfo_init);
+#endif
+
 # define DO(fn) long do_##fn
 
 #endif
-- 
2.26.1

[PATCH v8 08/12] xen: add /buildinfo/config entry to hypervisor filesystem

[Juergen Gross]

Add the /buildinfo/config entry to the hypervisor filesystem. This
entry contains the .config file used to build the hypervisor.

Signed-off-by: Juergen Gross 
---
V3:
- store data in gzip format
- use binfile mechanism to create data file
- move code to kernel.c

V6:
- add config item for the /buildinfo/config (Jan Beulich)
- make config related variables const in kernel.h (Jan Beulich)

V7:
- update doc (Jan Beulich)
- use "rm -f" in Makefile (Jan Beulich)

V8:
- add dependency top CONFIG_HYPFS
- use macro for definition of leaf (Jan Beulich)

Signed-off-by: Juergen Gross 
---
 .gitignore   |  2 ++
 docs/misc/hypfs-paths.pandoc |  4 
 xen/common/Kconfig   | 11 +++
 xen/common/Makefile  | 12 
 xen/common/kernel.c  | 11 +++
 xen/include/xen/kernel.h |  3 +++
 6 files changed, 43 insertions(+)

diff --git a/.gitignore b/.gitignore
index ce3ef23d45..a58de1fd4a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -298,6 +298,8 @@ xen/arch/*/efi/boot.c
 xen/arch/*/efi/compat.c
 xen/arch/*/efi/efi.h
 xen/arch/*/efi/runtime.c
+xen/common/config_data.S
+xen/common/config.gz
 xen/include/headers*.chk
 xen/include/asm
 xen/include/asm-*/asm-offsets.h
diff --git a/docs/misc/hypfs-paths.pandoc b/docs/misc/hypfs-paths.pandoc
index d730caf394..9a76bc383b 100644
--- a/docs/misc/hypfs-paths.pandoc
+++ b/docs/misc/hypfs-paths.pandoc
@@ -135,6 +135,10 @@ Information about the compile domain.
 
 The compiler used to build Xen.
 
+ /buildinfo/config = STRING [CONFIG_HYPFS_CONFIG]
+
+The contents of the `xen/.config` file at the time of the hypervisor build.
+
  /buildinfo/version/
 
 A directory containing version information of the hypervisor.
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 2515e053c8..b36cd74a24 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -127,6 +127,17 @@ config HYPFS
 
  If unsure, say Y.
 
+config HYPFS_CONFIG
+   bool "Provide hypervisor .config via hypfs entry"
+   default y
+   depends on HYPFS
+   ---help---
+ When enabled the contents of the .config file used to build the
+ hypervisor are provided via the hypfs entry /buildinfo/config.
+
+ Disable this option in case you want to spare some memory or you
+ want to hide the .config contents from dom0.
+
 config KEXEC
bool "kexec support"
default y
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 98b7904dcd..d6d7bad2a9 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -1,6 +1,7 @@
 obj-$(CONFIG_ARGO) += argo.o
 obj-y += bitmap.o
 obj-y += bsearch.o
+obj-$(CONFIG_HYPFS_CONFIG) += config_data.o
 obj-$(CONFIG_CORE_PARKING) += core_parking.o
 obj-y += cpu.o
 obj-$(CONFIG_DEBUG_TRACE) += debugtrace.o
@@ -73,3 +74,14 @@ obj-$(CONFIG_UBSAN) += ubsan/
 
 obj-$(CONFIG_NEEDS_LIBELF) += libelf/
 obj-$(CONFIG_HAS_DEVICE_TREE) += libfdt/
+
+config.gz: ../.config
+   gzip -c $< >$@
+
+config_data.o: config.gz
+
+config_data.S: $(XEN_ROOT)/xen/tools/binfile
+   $(XEN_ROOT)/xen/tools/binfile $@ config.gz xen_config_data
+
+clean::
+   rm -f config_data.S config.gz 2>/dev/null
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index db7bd23fcb..f8f41820d5 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -390,6 +390,10 @@ static HYPFS_STRING_INIT(compile_date, "compile_date");
 static HYPFS_STRING_INIT(compile_domain, "compile_domain");
 static HYPFS_STRING_INIT(extra, "extra");
 
+#ifdef CONFIG_HYPFS_CONFIG
+static HYPFS_STRING_INIT(config, "config");
+#endif
+
 static int __init buildinfo_init(void)
 {
 hypfs_add_dir(_root, , true);
@@ -415,6 +419,13 @@ static int __init buildinfo_init(void)
 hypfs_add_leaf(, , true);
 hypfs_add_leaf(, , true);
 
+#ifdef CONFIG_HYPFS_CONFIG
+config.e.encoding = XEN_HYPFS_ENC_GZIP;
+config.e.size = xen_config_data_size;
+config.content = _config_data;
+hypfs_add_leaf(, , true);
+#endif
+
 return 0;
 }
 __initcall(buildinfo_init);
diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h
index 548b64da9f..02e3281f52 100644
--- a/xen/include/xen/kernel.h
+++ b/xen/include/xen/kernel.h
@@ -100,5 +100,8 @@ extern enum system_state {
 
 bool_t is_active_kernel_text(unsigned long addr);
 
+extern const char xen_config_data;
+extern const unsigned int xen_config_data_size;
+
 #endif /* _LINUX_KERNEL_H */
 
-- 
2.26.1

[PATCH v8 06/12] tools: add xenfs tool

[Juergen Gross]

Add the xenfs tool for accessing the hypervisor filesystem.

Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
---
V1:
- rename to xenhypfs
- don't use "--" for subcommands
- add write support

V2:
- escape non-printable characters per default with cat subcommand
  (Ian Jackson)
- add -b option to cat subcommand (Ian Jackson)
- add man page

V3:
- adapt to new hypfs interface

V7:
- added missing bool for ls output

Signed-off-by: Juergen Gross 
---
 .gitignore  |   1 +
 docs/man/xenhypfs.1.pod |  61 +
 tools/misc/Makefile |   6 ++
 tools/misc/xenhypfs.c   | 192 
 4 files changed, 260 insertions(+)
 create mode 100644 docs/man/xenhypfs.1.pod
 create mode 100644 tools/misc/xenhypfs.c

diff --git a/.gitignore b/.gitignore
index 41b1d4c8a1..ce3ef23d45 100644
--- a/.gitignore
+++ b/.gitignore
@@ -368,6 +368,7 @@ tools/libxl/test_timedereg
 tools/libxl/test_fdderegrace
 tools/firmware/etherboot/eb-roms.h
 tools/firmware/etherboot/gpxe-git-snapshot.tar.gz
+tools/misc/xenhypfs
 tools/misc/xenwatchdogd
 tools/misc/xen-hvmcrash
 tools/misc/xen-lowmemd
diff --git a/docs/man/xenhypfs.1.pod b/docs/man/xenhypfs.1.pod
new file mode 100644
index 00..37aa488fcc
--- /dev/null
+++ b/docs/man/xenhypfs.1.pod
@@ -0,0 +1,61 @@
+=head1 NAME
+
+xenhypfs - Xen tool to access Xen hypervisor file system
+
+=head1 SYNOPSIS
+
+B I [I] [I]
+
+=head1 DESCRIPTION
+
+The B program is used to access the Xen hypervisor file system.
+It can be used to show the available entries, to show their contents and
+(if allowed) to modify their contents.
+
+=head1 SUBCOMMANDS
+
+=over 4
+
+=item B I
+
+List the available entries below I.
+
+=item B [I<-b>] I
+
+Show the contents of the entry specified by I. Non-printable characters
+other than white space characters (like tab, new line) will be shown as
+B<\xnn> (B being a two digit hex number) unless the option B<-b> is
+specified.
+
+=item B I I
+
+Set the contents of the entry specified by I to I.
+
+=item B
+
+Show all the entries of the file system as a tree.
+
+=back
+
+=head1 RETURN CODES
+
+=over 4
+
+=item B<0>
+
+Success
+
+=item B<1>
+
+Invalid usage (e.g. unknown subcommand, unknown option, missing parameter).
+
+=item B<2>
+
+Entry not found while traversing the tree.
+
+=item B<3>
+
+Access right violation.
+
+=back
+
diff --git a/tools/misc/Makefile b/tools/misc/Makefile
index 63947bfadc..9fdb13597f 100644
--- a/tools/misc/Makefile
+++ b/tools/misc/Makefile
@@ -24,6 +24,7 @@ INSTALL_SBIN-$(CONFIG_X86) += xen-lowmemd
 INSTALL_SBIN-$(CONFIG_X86) += xen-mfndump
 INSTALL_SBIN-$(CONFIG_X86) += xen-ucode
 INSTALL_SBIN   += xencov
+INSTALL_SBIN   += xenhypfs
 INSTALL_SBIN   += xenlockprof
 INSTALL_SBIN   += xenperf
 INSTALL_SBIN   += xenpm
@@ -86,6 +87,9 @@ xenperf: xenperf.o
 xenpm: xenpm.o
$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
 
+xenhypfs: xenhypfs.o
+   $(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenhypfs) $(APPEND_LDFLAGS)
+
 xenlockprof: xenlockprof.o
$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
 
@@ -94,6 +98,8 @@ xen-hptool.o: CFLAGS += -I$(XEN_ROOT)/tools/libxc 
$(CFLAGS_libxencall)
 xen-hptool: xen-hptool.o
$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenevtchn) $(LDLIBS_libxenctrl) 
$(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(APPEND_LDFLAGS)
 
+xenhypfs.o: CFLAGS += $(CFLAGS_libxenhypfs)
+
 # xen-mfndump incorrectly uses libxc internals
 xen-mfndump.o: CFLAGS += -I$(XEN_ROOT)/tools/libxc $(CFLAGS_libxencall)
 xen-mfndump: xen-mfndump.o
diff --git a/tools/misc/xenhypfs.c b/tools/misc/xenhypfs.c
new file mode 100644
index 00..158b901f42
--- /dev/null
+++ b/tools/misc/xenhypfs.c
@@ -0,0 +1,192 @@
+#define _GNU_SOURCE
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static struct xenhypfs_handle *hdl;
+
+static int usage(void)
+{
+fprintf(stderr, "usage: xenhypfs ls \n");
+fprintf(stderr, "   xenhypfs cat [-b] \n");
+fprintf(stderr, "   xenhypfs write  \n");
+fprintf(stderr, "   xenhypfs tree\n");
+
+return 1;
+}
+
+static void xenhypfs_print_escaped(char *string)
+{
+char *c;
+
+for (c = string; *c; c++) {
+if (isgraph(*c) || isspace(*c))
+printf("%c", *c);
+else
+printf("\\x%02x", *c);
+}
+printf("\n");
+}
+
+static int xenhypfs_cat(int argc, char *argv[])
+{
+int ret = 0;
+char *result;
+char *path;
+bool bin = false;
+
+switch (argc) {
+case 1:
+path = argv[0];
+break;
+
+case 2:
+if (strcmp(argv[0], "-b"))
+return usage();
+bin = true;
+path = argv[1];
+break;
+
+default:
+return usage();
+}
+
+result = xenhypfs_read(hdl, path);
+if (!result) {
+perror("could not read");
+ret = 3;
+} else {
+if (!bin)
+

Re: [PATCH] tools/libxc: Reduce feature handling complexity in xc_cpuid_apply_policy()

[Andrew Cooper]

Tools ping?

~Andrew

On 03/03/2020 18:23, Andrew Cooper wrote:
> xc_cpuid_apply_policy() is gaining extra parameters to untangle CPUID
> complexity in Xen.  While an improvement in general, it does have the
> unfortunate side effect of duplicating some settings across muliple
> parameters.
>
> Rearrange the logic to only consider 'pae' if no explicit featureset is
> provided.  This reduces the complexity for callers who have already provided a
> pae setting in the featureset.
>
> Signed-off-by: Andrew Cooper 
> ---
> CC: Jan Beulich 
> CC: Wei Liu 
> CC: Roger Pau Monné 
> CC: Ian Jackson 
> ---
>  tools/libxc/include/xenctrl.h | 6 ++
>  tools/libxc/xc_cpuid_x86.c| 7 +--
>  2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index fc6e57a1a0..8d13a7e20b 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -1798,6 +1798,12 @@ int xc_cpuid_set(xc_interface *xch,
>   const unsigned int *input,
>   const char **config,
>   char **config_transformed);
> +/*
> + * Make adjustments to the CPUID settings for a domain.
> + *
> + * Either pass a full new @featureset (and @nr_features), or adjust 
> individual
> + * features (@pae).
> + */
>  int xc_cpuid_apply_policy(xc_interface *xch,
>uint32_t domid,
>const uint32_t *featureset,
> diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
> index 5ced6d18b9..f045b03223 100644
> --- a/tools/libxc/xc_cpuid_x86.c
> +++ b/tools/libxc/xc_cpuid_x86.c
> @@ -532,6 +532,11 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t 
> domid,
>  
>  cpuid_featureset_to_policy(feat, p);
>  }
> +else
> +{
> +if ( di.hvm )
> +p->basic.pae = pae;
> +}
>  
>  if ( !di.hvm )
>  {
> @@ -615,8 +620,6 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t 
> domid,
>  break;
>  }
>  
> -p->basic.pae = pae;
> -
>  /*
>   * These settings are necessary to cause earlier HVM_PARAM_NESTEDHVM 
> /
>   * XEN_DOMCTL_disable_migrate settings to be reflected correctly in

[PATCH] x86/gen-cpuid: Distinguish default vs max in feature annotations

[Andrew Cooper]

Allow lowercase a/s/h to be used to annotate a non-default feature.

However, until the toolstack migration logic is fixed, it is not safe to
activate yet.  Tolerate the annotations, but ignore them for now.

Signed-off-by: Andrew Cooper 
Reviewed-by: Jan Beulich 
---
CC: Jan Beulich 
CC: Wei Liu 
CC: Roger Pau Monné 

This patch has been pending the toolstack work for several months now, and we
want to start using the max tags for x86 emul work.
---
 xen/include/public/arch-x86/cpufeatureset.h | 2 ++
 xen/tools/gen-cpuid.py  | 5 +
 2 files changed, 7 insertions(+)

diff --git a/xen/include/public/arch-x86/cpufeatureset.h 
b/xen/include/public/arch-x86/cpufeatureset.h
index e2749245f3..0ffab6c57b 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -87,6 +87,8 @@ enum {
  *   'A' = All guests.
  *   'S' = All HVM guests (not PV guests).
  *   'H' = HVM HAP guests (not PV or HVM Shadow guests).
+ *   Upper case => Available by default
+ *   Lower case => Can be opted-in to, but not available by default.
  */
 
 /* Intel-defined CPU features, CPUID level 0x0001.edx, word 0 */
diff --git a/xen/tools/gen-cpuid.py b/xen/tools/gen-cpuid.py
index af5610a5e6..d90a2d85c7 100755
--- a/xen/tools/gen-cpuid.py
+++ b/xen/tools/gen-cpuid.py
@@ -23,6 +23,7 @@ def __init__(self, input, output):
 self.raw = {
 '!': set(),
 'A': set(), 'S': set(), 'H': set(),
+'a': set(), 's': set(), 'h': set(),
 }
 
 # State calculated
@@ -133,9 +134,13 @@ def crunch_numbers(state):
 state.hvm_shadow_def = state.pv_def | state.raw['S']
 state.hvm_hap_def = state.hvm_shadow_def | state.raw['H']
 
+# TODO: Ignore def/max split until the toolstack migration logic is fixed
 state.pv_max = state.pv_def
 state.hvm_shadow_max = state.hvm_shadow_def
 state.hvm_hap_max = state.hvm_hap_def
+# state.pv_max = state.raw['A'] | state.raw['a']
+# state.hvm_shadow_max = state.pv_max | state.raw['S'] | state.raw['s']
+# state.hvm_hap_max = state.hvm_shadow_max | state.raw['H'] | 
state.raw['h']
 
 #
 # Feature dependency information.
-- 
2.11.0

Re: [PATCH] x86/PVH: PHYSDEVOP_pci_mmcfg_reserved should not blindly register a region

[Jan Beulich]

On 08.05.2020 17:03, Roger Pau Monné wrote:
> On Fri, May 08, 2020 at 02:43:38PM +0200, Jan Beulich wrote:
>> --- a/xen/arch/x86/hvm/io.c
>> +++ b/xen/arch/x86/hvm/io.c
>> @@ -558,6 +558,47 @@ int register_vpci_mmcfg_handler(struct d
>>  return 0;
>>  }
>>  
>> +int unregister_vpci_mmcfg_handler(struct domain *d, paddr_t addr,
>> +  unsigned int start_bus, unsigned int 
>> end_bus,
>> +  unsigned int seg)
>> +{
>> +struct hvm_mmcfg *mmcfg;
>> +int rc = -ENOENT;
>> +
>> +ASSERT(is_hardware_domain(d));
>> +
>> +if ( start_bus > end_bus )
>> +return -EINVAL;
>> +
>> +write_lock(>arch.hvm.mmcfg_lock);
>> +
>> +list_for_each_entry ( mmcfg, >arch.hvm.mmcfg_regions, next )
>> +if ( mmcfg->addr == addr + (start_bus << 20) &&
>> + mmcfg->segment == seg &&
>> + mmcfg->start_bus == start_bus &&
>> + mmcfg->size == ((end_bus - start_bus + 1) << 20) )
>> +{
>> +list_del(>next);
>> +if ( !list_empty(>arch.hvm.mmcfg_regions) )
>> +xfree(mmcfg);
>> +else
>> +{
>> +/*
>> + * Cannot unregister the MMIO handler - leave a fake entry
>> + * on the list.
>> + */
>> +memset(mmcfg, 0, sizeof(*mmcfg));
>> +list_add(>next, >arch.hvm.mmcfg_regions);
> 
> Instead of leaving this zombie entry around maybe we could add a
> static bool in register_vpci_mmcfg_handler to signal whether the MMIO
> intercept has been registered?

That was my initial plan indeed, but registration is per-domain.

>> --- a/xen/arch/x86/physdev.c
>> +++ b/xen/arch/x86/physdev.c
>> @@ -559,12 +559,18 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H
>>  if ( !ret && has_vpci(currd) )
>>  {
>>  /*
>> - * For HVM (PVH) domains try to add the newly found MMCFG to the
>> - * domain.
>> + * For HVM (PVH) domains try to add/remove the reported MMCFG
>> + * to/from the domain.
>>   */
>> -ret = register_vpci_mmcfg_handler(currd, info.address,
>> -  info.start_bus, info.end_bus,
>> -  info.segment);
>> +if ( info.flags & XEN_PCI_MMCFG_RESERVED )
> 
> Do you think you could also add a small note in physdev.h regarding
> the fact that XEN_PCI_MMCFG_RESERVED is used to register a MMCFG
> region, and not setting it would imply an unregister request?
> 
> It's not obvious to me from the name of the flag.

The main purpose of the flag is to identify whether a region can be
used (because of having been found marked suitably reserved by
firmware). The flag not set effectively means "region is not marked
reserved". You pointing this out makes me wonder whether instead I
should simply expand the if() in context, without making it behave
like unregistration. Then again we'd have no way to unregister a
region, and hence (ab)using this function for this purpose seems to
makes sense (and, afaict, not require any code changes elsewhere).

Jan

Re: [PATCH v8 04/12] x86emul: support SERIALIZE

[Andrew Cooper]

On 08/05/2020 14:59, Jan Beulich wrote:
> On 08.05.2020 15:00, Andrew Cooper wrote:
>> On 08/05/2020 08:34, Jan Beulich wrote:
> @@ -5660,6 +5661,18 @@ x86_emulate(
>  goto done;
>  break;
>  
> +case 0xe8:
> +switch ( vex.pfx )
> +{
> +case vex_none: /* serialize */
> +host_and_vcpu_must_have(serialize);
> +asm volatile ( ".byte 0x0f, 0x01, 0xe8" );
 There is very little need for an actual implementation here.  The VMExit
 to get here is good enough.

 The only question is whether pre-unrestricted_guest Intel boxes are
 liable to find this in real mode code.
>>> Apart from this we also shouldn't assume HVM in the core emulator,
>>> I think.
>> It's not assuming HVM.  Its just that there is no way we can end up
>> emulating this instruction from PV context.
>>
>> If we could end up here in PV context, the exception causing us to
>> emulate to begin with would be good enough as well.
> With the current way of where/how emulation gets involved -
> yes. I'd like to remind you though of the 4-insn window
> shadow code tries to emulate over for PAE guests. There
> is no intervening #VMEXIT there.
>
> So do you want me to drop the asm() then, and switch from
> host_and_vcpu_must_have(serialize) to just
> vcpu_must_have(serialize)?

No - keep it as is.  This isn't a fastpath, and it is much safer to
assume there might be something we've forgotten.  (Or perhaps some new
future behaviour included in the definition of architecturally serialising).

~Andrew

Re: [PATCH v8 08/12] x86emul: support FLDENV and FRSTOR

[Jan Beulich]

On 08.05.2020 15:37, Roger Pau Monné wrote:
> On Tue, May 05, 2020 at 10:16:20AM +0200, Jan Beulich wrote:
>> --- a/tools/tests/x86_emulator/test_x86_emulator.c
>> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
>> @@ -2442,6 +2442,27 @@ int main(int argc, char **argv)
>>  else
>>  printf("skipped\n");
>>  
>> +printf("%-40s", "Testing fldenv 8(%edx)...");
> 
> Likely a stupid question, but why the added 8? edx will contain the
> memory address used to save the sate by fnstenv, so I would expect
> fldenv to just load from there?

The 8 is just to vary ModR/M encodings acrosss the various
tests - it's an arbitrary choice.

>> +if ( stack_exec && cpu_has_fpu )
>> +{
>> +asm volatile ( "fnstenv %0\n\t"
>> +   "fninit"
>> +   : "=m" (res[2]) :: "memory" );
> 
> Why do you need the memory clobber here? I assume it's because res is
> of type unsigned int and hence doesn't have the right size that
> fnstenv will actually write to?

Correct.

>> @@ -4948,21 +4949,14 @@ x86_emulate(
>>  dst.bytes = 4;
>>  emulate_fpu_insn_memdst(b, modrm_reg & 7, dst.val);
>>  break;
>> -case 4: /* fldenv - TODO */
>> -state->fpu_ctrl = true;
>> -goto unimplemented_insn;
>> -case 5: /* fldcw m2byte */
>> -state->fpu_ctrl = true;
>> -fpu_memsrc16:
>> -if ( (rc = ops->read(ea.mem.seg, ea.mem.off, ,
>> - 2, ctxt)) != X86EMUL_OKAY )
>> -goto done;
>> -emulate_fpu_insn_memsrc(b, modrm_reg & 7, src.val);
>> -break;
>> +case 4: /* fldenv */
>> +/* Raise #MF now if there are pending unmasked exceptions. 
>> */
>> +emulate_fpu_insn_stub(0xd9, 0xd0 /* fnop */);
> 
> Maybe it would make sense to have a wrapper for fnop?

I'm not convinced that would be worth it.

>> @@ -11648,6 +11651,89 @@ int x86_emul_blk(
>>  
>>  #ifndef X86EMUL_NO_FPU
>>  
>> +case blk_fld:
>> +ASSERT(!data);
>> +
>> +/* state->rex_prefix carries CR0.PE && !EFLAGS.VM setting */
>> +switch ( bytes )
>> +{
>> +case sizeof(fpstate.env):
>> +case sizeof(fpstate):
>> +memcpy(, ptr, sizeof(fpstate.env));
>> +if ( !state->rex_prefix )
>> +{
>> +unsigned int fip = fpstate.env.mode.real.fip_lo +
>> +   (fpstate.env.mode.real.fip_hi << 16);
>> +unsigned int fdp = fpstate.env.mode.real.fdp_lo +
>> +   (fpstate.env.mode.real.fdp_hi << 16);
>> +unsigned int fop = fpstate.env.mode.real.fop;
>> +
>> +fpstate.env.mode.prot.fip = fip & 0xf;
>> +fpstate.env.mode.prot.fcs = fip >> 4;
>> +fpstate.env.mode.prot.fop = fop;
>> +fpstate.env.mode.prot.fdp = fdp & 0xf;
>> +fpstate.env.mode.prot.fds = fdp >> 4;
> 
> I've found the layouts in the SDM vol. 1, but I haven't been able to
> found the translation mechanism from real to protected. Could you
> maybe add a reference here?

A reference to some piece of documentation? I don't think this
is spelled out anywhere. It's also only one of various possible
ways of doing the translation, but among them the most flexible
one for possible consumers of the data (because of using the
smallest possible offsets into the segments).

>> +}
>> +
>> +if ( bytes == sizeof(fpstate.env) )
>> +ptr = NULL;
>> +else
>> +ptr += sizeof(fpstate.env);
>> +break;
>> +
>> +case sizeof(struct x87_env16):
>> +case sizeof(struct x87_env16) + sizeof(fpstate.freg):
>> +{
>> +const struct x87_env16 *env = ptr;
>> +
>> +fpstate.env.fcw = env->fcw;
>> +fpstate.env.fsw = env->fsw;
>> +fpstate.env.ftw = env->ftw;
>> +
>> +if ( state->rex_prefix )
>> +{
>> +fpstate.env.mode.prot.fip = env->mode.prot.fip;
>> +fpstate.env.mode.prot.fcs = env->mode.prot.fcs;
>> +fpstate.env.mode.prot.fdp = env->mode.prot.fdp;
>> +fpstate.env.mode.prot.fds = env->mode.prot.fds;
>> +fpstate.env.mode.prot.fop = 0; /* unknown */
>> +}
>> +else
>> +{
>> +unsigned int fip = env->mode.real.fip_lo +
>> +   (env->mode.real.fip_hi << 16);
>> +unsigned int fdp = env->mode.real.fdp_lo +
>> +   (env->mode.real.fdp_hi << 16);
>> +unsigned int fop = env->mode.real.fop;
>> +
>> +fpstate.env.mode.prot.fip = fip & 0xf;
>> +fpstate.env.mode.prot.fcs = fip >> 4;
>> +

Re: [PATCH] x86/PVH: PHYSDEVOP_pci_mmcfg_reserved should not blindly register a region

[Roger Pau Monné]

On Fri, May 08, 2020 at 02:43:38PM +0200, Jan Beulich wrote:
> The op has a register/unregister flag, and hence registration shouldn't
> happen unilaterally. Introduce unregister_vpci_mmcfg_handler() to handle
> this case.
> 
> Fixes: eb3dd90e4089 ("x86/physdev: enable PHYSDEVOP_pci_mmcfg_reserved for 
> PVH Dom0")
> Signed-off-by: Jan Beulich 
> 
> --- a/xen/arch/x86/hvm/io.c
> +++ b/xen/arch/x86/hvm/io.c
> @@ -558,6 +558,47 @@ int register_vpci_mmcfg_handler(struct d
>  return 0;
>  }
>  
> +int unregister_vpci_mmcfg_handler(struct domain *d, paddr_t addr,
> +  unsigned int start_bus, unsigned int 
> end_bus,
> +  unsigned int seg)
> +{
> +struct hvm_mmcfg *mmcfg;
> +int rc = -ENOENT;
> +
> +ASSERT(is_hardware_domain(d));
> +
> +if ( start_bus > end_bus )
> +return -EINVAL;
> +
> +write_lock(>arch.hvm.mmcfg_lock);
> +
> +list_for_each_entry ( mmcfg, >arch.hvm.mmcfg_regions, next )
> +if ( mmcfg->addr == addr + (start_bus << 20) &&
> + mmcfg->segment == seg &&
> + mmcfg->start_bus == start_bus &&
> + mmcfg->size == ((end_bus - start_bus + 1) << 20) )
> +{
> +list_del(>next);
> +if ( !list_empty(>arch.hvm.mmcfg_regions) )
> +xfree(mmcfg);
> +else
> +{
> +/*
> + * Cannot unregister the MMIO handler - leave a fake entry
> + * on the list.
> + */
> +memset(mmcfg, 0, sizeof(*mmcfg));
> +list_add(>next, >arch.hvm.mmcfg_regions);

Instead of leaving this zombie entry around maybe we could add a
static bool in register_vpci_mmcfg_handler to signal whether the MMIO
intercept has been registered?

> +}
> +rc = 0;
> +break;
> +}
> +
> +write_unlock(>arch.hvm.mmcfg_lock);
> +
> +return rc;
> +}
> +
>  void destroy_vpci_mmcfg(struct domain *d)
>  {
>  struct list_head *mmcfg_regions = >arch.hvm.mmcfg_regions;
> --- a/xen/arch/x86/physdev.c
> +++ b/xen/arch/x86/physdev.c
> @@ -559,12 +559,18 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H
>  if ( !ret && has_vpci(currd) )
>  {
>  /*
> - * For HVM (PVH) domains try to add the newly found MMCFG to the
> - * domain.
> + * For HVM (PVH) domains try to add/remove the reported MMCFG
> + * to/from the domain.
>   */
> -ret = register_vpci_mmcfg_handler(currd, info.address,
> -  info.start_bus, info.end_bus,
> -  info.segment);
> +if ( info.flags & XEN_PCI_MMCFG_RESERVED )

Do you think you could also add a small note in physdev.h regarding
the fact that XEN_PCI_MMCFG_RESERVED is used to register a MMCFG
region, and not setting it would imply an unregister request?

It's not obvious to me from the name of the flag.

Thanks, Roger.

Re: [PATCH] x86/PVH: PHYSDEVOP_pci_mmcfg_reserved should not blindly register a region

[Roger Pau Monné]

On Fri, May 08, 2020 at 03:49:35PM +0200, Jan Beulich wrote:
> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments 
> unless you have verified the sender and know the content is safe.
> 
> On 08.05.2020 14:54, Andrew Cooper wrote:
> > On 08/05/2020 13:43, Jan Beulich wrote:
> >> The op has a register/unregister flag, and hence registration shouldn't
> >> happen unilaterally. Introduce unregister_vpci_mmcfg_handler() to handle
> >> this case.
> >>
> >> Fixes: eb3dd90e4089 ("x86/physdev: enable PHYSDEVOP_pci_mmcfg_reserved for 
> >> PVH Dom0")
> >> Signed-off-by: Jan Beulich 
> > 
> > I agree in principle that registration shouldn't be unilateral, but why
> > on earth does the API behave like that to begin with?
> > 
> > There is no provision to move or update MMCFG regions in any spec I'm
> > aware of, and hardware cannot in practice update memory routing like this.
> > 
> > Under what circumstances should we tolerate an unregister in the first
> > place?
> 
> Hot unplug of an entire segment, for example.

An OS could also rebalance the resources of a host bridge, according
to the PCI firmware spec, in which case _CBA should be re-evaluated.

I'm not sure whether rebalancing would work anyway, or if there even
are systems that support this and OSes that would attempt to do it,
but since we have the interface for this let's try to do something
sensible.

The other options is simply returning -EOPNOTSUPP. Iff the domain
doesn't try to access devices that would reside on the segment
hot-unplugged it shouldn't make much of a difference, rebalancing is
the case were Xen must support add/remove in order to re-place the
position of the ECAM areas.

Thanks, Roger.

Re: Xen Coding style

[Jürgen Groß]

On 08.05.20 16:23, Tamas K Lengyel wrote:

On Fri, May 8, 2020 at 8:18 AM Jürgen Groß  wrote:


On 08.05.20 14:55, Tamas K Lengyel wrote:

On Fri, May 8, 2020 at 6:21 AM Julien Grall  wrote:


Hi Jan,

On 08/05/2020 12:20, Jan Beulich wrote:

On 08.05.2020 12:00, Julien Grall wrote:

You seem to be the maintainer with the most unwritten rules. Would
you mind to have a try at writing a coding style based on it?


On the basis that even small, single aspect patches to CODING_STYLE
have been ignored [1],


Your thread is one of the example why I started this thread. Agreeing on
specific rule doesn't work because it either result to bikesheding or
there is not enough interest to review rule by rule.


I don't think this would be a good use of my
time.


I would have assumed that the current situation (i.e
nitpicking/bikeshedding on the ML) is not a good use of your time :).

I would be happy to put some effort to help getting the coding style
right, however I believe focusing on an overall coding style would value
everyone's time better than a rule by rule discussion.


If I was promised (reasonable) feedback, I could take what I
have and try to add at least a few more things based on what I find
myself commenting on more frequently. But really I'd prefer it to
be done the other way around - for people to look at the patches
already sent, and for me to only subsequently send more. After all,
if already those adjustments are controversial, I don't think we
could settle on others.

While I understand this requires another investment from your part, I am
afraid it is going to be painful for someone else to go through all the
existing coding style bikeshedding and infer your unwritten rules.

It might be more beneficial for that person to pursue the work done by
Tamas and Viktor in the past (see my previous e-mail). This may mean to
adopt an existing coding style (BSD) and then tweak it.


Thanks Julien for restarting this discussion. IMHO agreeing on a set
of style rules ahead and then applying universally all at once is not
going to be productive since we are so all over the place. Instead, I
would recommend we start piece-by-piece. We introduce a baseline style
checker, then maintainers can decide when and if they want to move
their code-base to be under the automated style checker. That way we
have a baseline and each maintainer can decide on their own term when
they want to have their files be also style checked and in what form.
The upside of this route I think is pretty clear: we can have at least
partial automation even while we figure out what to do with some of
the more problematic files and quirks that are in our code-base. I
would highly prefer this route since I would immediately bring all
files I maintain over to the automated checker just so I never ever
have to deal with this again manually. What style is in use to me
really doesn't matter, BSD was very close with some minor tweaks, or
even what we use to check the style just as long as we have
_something_.


Wouldn't it make more sense to have a patch checker instead and accept
only patches which change code according to the style guide? This
wouldn't require to change complete files at a time.


In theory, yes. But in practice this would require that we can agree
on a style that applies to all patches that touch any file within Xen.
We can't seem to do that because there are too many exceptions and
corner-cases and personal-preferences of maintainers that apply only
to a subset of the codebase. So AFAICT what you propose doesn't seem
to be a viable way to start.


I think long ago we already agreed to have a control file which tells a
style checker which style to apply (if any). As a start we could have a
patch checker checking only the commit message (has it a Signed-off-by:
etc.). The next step would be to add the control file, and the framework
to split the patch into the changed file hunks and passing each hunk to
the correct checking script (might be doing nothing in the beginning).
And then we could add some logic to the single checkers.


Juergen

Re: [PATCH v8 05/12] x86emul: support X{SUS,RES}LDTRK

[Jan Beulich]

On 08.05.2020 15:15, Andrew Cooper wrote:
> On 08/05/2020 08:38, Jan Beulich wrote:
>> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments 
>> unless you have verified the sender and know the content is safe.
>>
>> On 07.05.2020 22:13, Andrew Cooper wrote:
>>> On 05/05/2020 09:14, Jan Beulich wrote:
 --- a/xen/tools/gen-cpuid.py
 +++ b/xen/tools/gen-cpuid.py
 @@ -284,6 +284,9 @@ def crunch_numbers(state):
  # as dependent features simplifies Xen's logic, and prevents the 
 guest
  # from seeing implausible configurations.
  IBRSB: [STIBP, SSBD],
 +
 +# In principle the TSXLDTRK insns could also be considered 
 independent.
 +RTM: [TSXLDTRK],
>>> Why the link?  There is no relevant interaction AFAICT.
>> Do the insns make any sense without TSX? Anyway - hence the
>> comment, and if you're convinced the connection does not
>> need making, I'd be okay dropping it. I would assume though
>> that we'd better hide TSXLDTRK whenever we hide RTM, which
>> is most easily achieved by having a connection here.
> 
> Actually - that is a very good point.  I expect there will (or should)
> be an interaction with MSR_TSX_CTRL, as it has CPUID-hiding functionality.
> 
> For now, could I ask you to not expose this to guests in this patch?

As per our irc discussion, I'd make it 'a' then instead of 'A'.
Will need to wait for gen-cpuid.py to accept 'a' then.

> For the emulator side of things alone I think this is ok (although
> looking over it a second time, we could really do with a comment in the
> code explaining that we're never in an RTM region, hence the nop behaviour).

I've added

/*
 * We're never in a transactional region when coming here
 * - nothing else to do.
 */

to both paths.

> I'll follow up with Intel, and we can figure out the CPUID derivation
> details at a later point.
> 
> If you're happy with this plan, then A-by to save a round trip.

Thanks.

Jan

Re: Xen Coding style

[Tamas K Lengyel]

On Fri, May 8, 2020 at 8:18 AM Jürgen Groß  wrote:
>
> On 08.05.20 14:55, Tamas K Lengyel wrote:
> > On Fri, May 8, 2020 at 6:21 AM Julien Grall  wrote:
> >>
> >> Hi Jan,
> >>
> >> On 08/05/2020 12:20, Jan Beulich wrote:
> >>> On 08.05.2020 12:00, Julien Grall wrote:
>  You seem to be the maintainer with the most unwritten rules. Would
>  you mind to have a try at writing a coding style based on it?
> >>>
> >>> On the basis that even small, single aspect patches to CODING_STYLE
> >>> have been ignored [1],
> >>
> >> Your thread is one of the example why I started this thread. Agreeing on
> >> specific rule doesn't work because it either result to bikesheding or
> >> there is not enough interest to review rule by rule.
> >>
> >>> I don't think this would be a good use of my
> >>> time.
> >>
> >> I would have assumed that the current situation (i.e
> >> nitpicking/bikeshedding on the ML) is not a good use of your time :).
> >>
> >> I would be happy to put some effort to help getting the coding style
> >> right, however I believe focusing on an overall coding style would value
> >> everyone's time better than a rule by rule discussion.
> >>
> >>> If I was promised (reasonable) feedback, I could take what I
> >>> have and try to add at least a few more things based on what I find
> >>> myself commenting on more frequently. But really I'd prefer it to
> >>> be done the other way around - for people to look at the patches
> >>> already sent, and for me to only subsequently send more. After all,
> >>> if already those adjustments are controversial, I don't think we
> >>> could settle on others.
> >> While I understand this requires another investment from your part, I am
> >> afraid it is going to be painful for someone else to go through all the
> >> existing coding style bikeshedding and infer your unwritten rules.
> >>
> >> It might be more beneficial for that person to pursue the work done by
> >> Tamas and Viktor in the past (see my previous e-mail). This may mean to
> >> adopt an existing coding style (BSD) and then tweak it.
> >
> > Thanks Julien for restarting this discussion. IMHO agreeing on a set
> > of style rules ahead and then applying universally all at once is not
> > going to be productive since we are so all over the place. Instead, I
> > would recommend we start piece-by-piece. We introduce a baseline style
> > checker, then maintainers can decide when and if they want to move
> > their code-base to be under the automated style checker. That way we
> > have a baseline and each maintainer can decide on their own term when
> > they want to have their files be also style checked and in what form.
> > The upside of this route I think is pretty clear: we can have at least
> > partial automation even while we figure out what to do with some of
> > the more problematic files and quirks that are in our code-base. I
> > would highly prefer this route since I would immediately bring all
> > files I maintain over to the automated checker just so I never ever
> > have to deal with this again manually. What style is in use to me
> > really doesn't matter, BSD was very close with some minor tweaks, or
> > even what we use to check the style just as long as we have
> > _something_.
>
> Wouldn't it make more sense to have a patch checker instead and accept
> only patches which change code according to the style guide? This
> wouldn't require to change complete files at a time.

In theory, yes. But in practice this would require that we can agree
on a style that applies to all patches that touch any file within Xen.
We can't seem to do that because there are too many exceptions and
corner-cases and personal-preferences of maintainers that apply only
to a subset of the codebase. So AFAICT what you propose doesn't seem
to be a viable way to start.

Tamas

Re: Xen Coding style

[Jürgen Groß]

On 08.05.20 14:55, Tamas K Lengyel wrote:

On Fri, May 8, 2020 at 6:21 AM Julien Grall  wrote:


Hi Jan,

On 08/05/2020 12:20, Jan Beulich wrote:

On 08.05.2020 12:00, Julien Grall wrote:

You seem to be the maintainer with the most unwritten rules. Would
you mind to have a try at writing a coding style based on it?


On the basis that even small, single aspect patches to CODING_STYLE
have been ignored [1],


Your thread is one of the example why I started this thread. Agreeing on
specific rule doesn't work because it either result to bikesheding or
there is not enough interest to review rule by rule.


I don't think this would be a good use of my
time.


I would have assumed that the current situation (i.e
nitpicking/bikeshedding on the ML) is not a good use of your time :).

I would be happy to put some effort to help getting the coding style
right, however I believe focusing on an overall coding style would value
everyone's time better than a rule by rule discussion.


If I was promised (reasonable) feedback, I could take what I
have and try to add at least a few more things based on what I find
myself commenting on more frequently. But really I'd prefer it to
be done the other way around - for people to look at the patches
already sent, and for me to only subsequently send more. After all,
if already those adjustments are controversial, I don't think we
could settle on others.

While I understand this requires another investment from your part, I am
afraid it is going to be painful for someone else to go through all the
existing coding style bikeshedding and infer your unwritten rules.

It might be more beneficial for that person to pursue the work done by
Tamas and Viktor in the past (see my previous e-mail). This may mean to
adopt an existing coding style (BSD) and then tweak it.


Thanks Julien for restarting this discussion. IMHO agreeing on a set
of style rules ahead and then applying universally all at once is not
going to be productive since we are so all over the place. Instead, I
would recommend we start piece-by-piece. We introduce a baseline style
checker, then maintainers can decide when and if they want to move
their code-base to be under the automated style checker. That way we
have a baseline and each maintainer can decide on their own term when
they want to have their files be also style checked and in what form.
The upside of this route I think is pretty clear: we can have at least
partial automation even while we figure out what to do with some of
the more problematic files and quirks that are in our code-base. I
would highly prefer this route since I would immediately bring all
files I maintain over to the automated checker just so I never ever
have to deal with this again manually. What style is in use to me
really doesn't matter, BSD was very close with some minor tweaks, or
even what we use to check the style just as long as we have
_something_.


Wouldn't it make more sense to have a patch checker instead and accept
only patches which change code according to the style guide? This
wouldn't require to change complete files at a time.


Juergen

Re: [PATCH v8 04/12] x86emul: support SERIALIZE

[Jan Beulich]

On 08.05.2020 15:00, Andrew Cooper wrote:
> On 08/05/2020 08:34, Jan Beulich wrote:
 @@ -5660,6 +5661,18 @@ x86_emulate(
  goto done;
  break;
  
 +case 0xe8:
 +switch ( vex.pfx )
 +{
 +case vex_none: /* serialize */
 +host_and_vcpu_must_have(serialize);
 +asm volatile ( ".byte 0x0f, 0x01, 0xe8" );
>>> There is very little need for an actual implementation here.  The VMExit
>>> to get here is good enough.
>>>
>>> The only question is whether pre-unrestricted_guest Intel boxes are
>>> liable to find this in real mode code.
>> Apart from this we also shouldn't assume HVM in the core emulator,
>> I think.
> 
> It's not assuming HVM.  Its just that there is no way we can end up
> emulating this instruction from PV context.
> 
> If we could end up here in PV context, the exception causing us to
> emulate to begin with would be good enough as well.

With the current way of where/how emulation gets involved -
yes. I'd like to remind you though of the 4-insn window
shadow code tries to emulate over for PAE guests. There
is no intervening #VMEXIT there.

So do you want me to drop the asm() then, and switch from
host_and_vcpu_must_have(serialize) to just
vcpu_must_have(serialize)?

Jan

Re: [PATCH] x86/PVH: PHYSDEVOP_pci_mmcfg_reserved should not blindly register a region

[Jan Beulich]

On 08.05.2020 14:54, Andrew Cooper wrote:
> On 08/05/2020 13:43, Jan Beulich wrote:
>> The op has a register/unregister flag, and hence registration shouldn't
>> happen unilaterally. Introduce unregister_vpci_mmcfg_handler() to handle
>> this case.
>>
>> Fixes: eb3dd90e4089 ("x86/physdev: enable PHYSDEVOP_pci_mmcfg_reserved for 
>> PVH Dom0")
>> Signed-off-by: Jan Beulich 
> 
> I agree in principle that registration shouldn't be unilateral, but why
> on earth does the API behave like that to begin with?
> 
> There is no provision to move or update MMCFG regions in any spec I'm
> aware of, and hardware cannot in practice update memory routing like this.
> 
> Under what circumstances should we tolerate an unregister in the first
> place?

Hot unplug of an entire segment, for example.

Jan

Re: [PATCH] x86/idle: prevent entering C6 with in service interrupts on Intel

[Jan Beulich]

On 07.05.2020 15:22, Roger Pau Monne wrote:
> Apply a workaround for Intel errata CLX30: "A Pending Fixed Interrupt
> May Be Dispatched Before an Interrupt of The Same Priority Completes".
> 
> It's not clear which models are affected, as the errata is listed in
> the "Second Generation Intel Xeon Scalable Processors" specification
> update, but the issue has been seen as far back as Nehalem processors.
> Apply the workaround to all Intel processors, the condition can be
> relaxed later.
> 
> Signed-off-by: Roger Pau Monné 
> ---
>  docs/misc/xen-command-line.pandoc |  8 
>  xen/arch/x86/acpi/cpu_idle.c  | 22 +-
>  xen/arch/x86/cpu/mwait-idle.c |  3 +++
>  xen/include/asm-x86/cpuidle.h |  1 +
>  4 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/misc/xen-command-line.pandoc 
> b/docs/misc/xen-command-line.pandoc
> index ee12b0f53f..6e868a2185 100644
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -652,6 +652,14 @@ Specify the size of the console debug trace buffer. By 
> specifying `cpu:`
>  additionally a trace buffer of the specified size is allocated per cpu.
>  The debug trace feature is only enabled in debugging builds of Xen.
>  
> +### disable-c6-isr
> +> `= `
> +
> +> Default: `true for Intel CPUs`
> +
> +Workaround for Intel errata CLX30. Prevent entering C6 idle states with in
> +service local APIC interrupts. Enabled by default for all Intel CPUs.
> +
>  ### dma_bits
>  > `= `
>  
> diff --git a/xen/arch/x86/acpi/cpu_idle.c b/xen/arch/x86/acpi/cpu_idle.c
> index b83446e77d..5023fea148 100644
> --- a/xen/arch/x86/acpi/cpu_idle.c
> +++ b/xen/arch/x86/acpi/cpu_idle.c
> @@ -573,6 +573,25 @@ static bool errata_c6_eoi_workaround(void)
>  return (fix_needed && cpu_has_pending_apic_eoi());
>  }
>  
> +static int8_t __read_mostly disable_c6_isr = -1;
> +boolean_param("disable-c6-isr", disable_c6_isr);
> +
> +/*
> + * Errata CLX30: A Pending Fixed Interrupt May Be Dispatched Before an
> + * Interrupt of The Same Priority Completes.
> + *
> + * Prevent entering C6 if there are pending lapic interrupts, or else the
> + * processor might dispatch further pending interrupts before the first one 
> has
> + * been completed.
> + */
> +bool errata_c6_isr_workaround(void)
> +{
> +if ( unlikely(disable_c6_isr == -1) )
> +disable_c6_isr = boot_cpu_data.x86_vendor == X86_VENDOR_INTEL;
> +
> +return disable_c6_isr && cpu_has_pending_apic_eoi();

This check being the same as in errata_c6_eoi_workaround(),
why don't you simply extend that function? (See also below.)
Also both variable and command line option descriptor could
go inside the function, to limit their scopes.

> @@ -676,7 +695,8 @@ static void acpi_processor_idle(void)
>  return;
>  }
>  
> -if ( (cx->type == ACPI_STATE_C3) && errata_c6_eoi_workaround() )
> +if ( (cx->type == ACPI_STATE_C3) &&
> + (errata_c6_eoi_workaround() || errata_c6_isr_workaround()) )
>  cx = power->safe_state;

I realize you only add to existing code, but I'm afraid this
existing code cannot be safely added to. Already prior to
your change there was a curious mismatch of C3 and c6 on this
line, and I don't see how ACPI_STATE_C3 correlates with
"core C6" state. Now this may have been the convention for
Nehalem/Westmere systems, but already the mwait-idle entries
for these CPU models have 4 entries (albeit such that they
match this scheme). As a result I think this at the very
least needs to be >=, not ==, even more so now that you want
to cover all Intel CPUs.

Obviously (I think) it is a mistake that mwait-idle doesn't
also activate this workaround, which imo is another reason to
stick to just errata_c6_eoi_workaround().

> --- a/xen/arch/x86/cpu/mwait-idle.c
> +++ b/xen/arch/x86/cpu/mwait-idle.c
> @@ -770,6 +770,9 @@ static void mwait_idle(void)
>   return;
>   }
>  
> + if (cx->type == ACPI_STATE_C3 && errata_c6_isr_workaround())
> + cx = power->safe_state;

Here it becomes even more relevant I think that == not be
used, as the static tables list deeper C-states; it's just
that the SKX table, which also gets used for CLX afaict, has
no entries beyond C6-SKX. Others with deeper ones presumably
have the deeper C-states similarly affected (or not) by this
erratum.

For reference, mwait_idle_cpu_init() has

hint = flg2MWAIT(cpuidle_state_table[cstate].flags);
state = MWAIT_HINT2CSTATE(hint) + 1;
...
cx->type = state;

i.e. the value you compare against is derived from the static
table entries. For Nehalem/Westmere this means that what goes
under ACPI_STATE_C3 is indeed C6-NHM, and this looks to match
for all the non-Atoms, but for none of the Atoms. Now Atoms
could easily be unaffected, but (just to take an example) if
C6-SNB was affected, surely C7-SNB would be, too.

Jan

Re: [PATCH v8 08/12] x86emul: support FLDENV and FRSTOR

[Roger Pau Monné]

On Tue, May 05, 2020 at 10:16:20AM +0200, Jan Beulich wrote:
> While the Intel SDM claims that FRSTOR itself may raise #MF upon
> completion, this was confirmed by Intel to be a doc error which will be
> corrected in due course; behavior is like FLDENV, and like old hard copy
> manuals describe it. Otherwise we'd have to emulate the insn by filling
> st(N) in suitable order, followed by FLDENV.
> 
> Signed-off-by: Jan Beulich 
> ---
> v7: New.
> 
> --- a/tools/tests/x86_emulator/test_x86_emulator.c
> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
> @@ -2442,6 +2442,27 @@ int main(int argc, char **argv)
>  else
>  printf("skipped\n");
>  
> +printf("%-40s", "Testing fldenv 8(%edx)...");

Likely a stupid question, but why the added 8? edx will contain the
memory address used to save the sate by fnstenv, so I would expect
fldenv to just load from there?

> +if ( stack_exec && cpu_has_fpu )
> +{
> +asm volatile ( "fnstenv %0\n\t"
> +   "fninit"
> +   : "=m" (res[2]) :: "memory" );

Why do you need the memory clobber here? I assume it's because res is
of type unsigned int and hence doesn't have the right size that
fnstenv will actually write to?

> +zap_fpsel([2], true);
> +instr[0] = 0xd9; instr[1] = 0x62; instr[2] = 0x08;
> +regs.eip = (unsigned long)[0];
> +regs.edx = (unsigned long)res;
> +rc = x86_emulate(, );
> +asm volatile ( "fnstenv %0" : "=m" (res[9]) :: "memory" );
> +if ( (rc != X86EMUL_OKAY) ||
> + memcmp(res + 2, res + 9, 28) ||
> + (regs.eip != (unsigned long)[3]) )
> +goto fail;
> +printf("okay\n");
> +}
> +else
> +printf("skipped\n");
> +
>  printf("%-40s", "Testing 16-bit fnsave (%ecx)...");
>  if ( stack_exec && cpu_has_fpu )
>  {
> @@ -2468,6 +2489,31 @@ int main(int argc, char **argv)
>  goto fail;
>  printf("okay\n");
>  }
> +else
> +printf("skipped\n");
> +
> +printf("%-40s", "Testing frstor (%edx)...");
> +if ( stack_exec && cpu_has_fpu )
> +{
> +const uint16_t seven = 7;
> +
> +asm volatile ( "fninit\n\t"
> +   "fld1\n\t"
> +   "fidivs %1\n\t"
> +   "fnsave %0\n\t"
> +   : "=" (res[0]) : "m" (seven) : "memory" );
> +zap_fpsel([0], true);
> +instr[0] = 0xdd; instr[1] = 0x22;
> +regs.eip = (unsigned long)[0];
> +regs.edx = (unsigned long)res;
> +rc = x86_emulate(, );
> +asm volatile ( "fnsave %0" : "=m" (res[27]) :: "memory" );
> +if ( (rc != X86EMUL_OKAY) ||
> + memcmp(res, res + 27, 108) ||
> + (regs.eip != (unsigned long)[2]) )
> +goto fail;
> +printf("okay\n");
> +}
>  else
>  printf("skipped\n");
>  
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -857,6 +857,7 @@ struct x86_emulate_state {
>  blk_NONE,
>  blk_enqcmd,
>  #ifndef X86EMUL_NO_FPU
> +blk_fld, /* FLDENV, FRSTOR */
>  blk_fst, /* FNSTENV, FNSAVE */
>  #endif
>  blk_movdir,
> @@ -4948,21 +4949,14 @@ x86_emulate(
>  dst.bytes = 4;
>  emulate_fpu_insn_memdst(b, modrm_reg & 7, dst.val);
>  break;
> -case 4: /* fldenv - TODO */
> -state->fpu_ctrl = true;
> -goto unimplemented_insn;
> -case 5: /* fldcw m2byte */
> -state->fpu_ctrl = true;
> -fpu_memsrc16:
> -if ( (rc = ops->read(ea.mem.seg, ea.mem.off, ,
> - 2, ctxt)) != X86EMUL_OKAY )
> -goto done;
> -emulate_fpu_insn_memsrc(b, modrm_reg & 7, src.val);
> -break;
> +case 4: /* fldenv */
> +/* Raise #MF now if there are pending unmasked exceptions. */
> +emulate_fpu_insn_stub(0xd9, 0xd0 /* fnop */);

Maybe it would make sense to have a wrapper for fnop?

> +/* fall through */
>  case 6: /* fnstenv */
>  fail_if(!ops->blk);
> -state->blk = blk_fst;
> -/* REX is meaningless for this insn by this point. */
> +state->blk = modrm_reg & 2 ? blk_fst : blk_fld;
> +/* REX is meaningless for these insns by this point. */
>  rex_prefix = in_protmode(ctxt, ops);
>  if ( (rc = ops->blk(ea.mem.seg, ea.mem.off, NULL,
>  op_bytes > 2 ? sizeof(struct x87_env32)
> @@ -4972,6 +4966,14 @@ x86_emulate(
>  goto done;
>  state->fpu_ctrl = true;
>  break;
> +case 5: /* fldcw m2byte */
> +state->fpu_ctrl = true;
> +

Re: [PATCH v8 05/12] x86emul: support X{SUS,RES}LDTRK

[Andrew Cooper]

On 08/05/2020 08:38, Jan Beulich wrote:
> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments 
> unless you have verified the sender and know the content is safe.
>
> On 07.05.2020 22:13, Andrew Cooper wrote:
>> On 05/05/2020 09:14, Jan Beulich wrote:
>>> --- a/xen/tools/gen-cpuid.py
>>> +++ b/xen/tools/gen-cpuid.py
>>> @@ -284,6 +284,9 @@ def crunch_numbers(state):
>>>  # as dependent features simplifies Xen's logic, and prevents the 
>>> guest
>>>  # from seeing implausible configurations.
>>>  IBRSB: [STIBP, SSBD],
>>> +
>>> +# In principle the TSXLDTRK insns could also be considered 
>>> independent.
>>> +RTM: [TSXLDTRK],
>> Why the link?  There is no relevant interaction AFAICT.
> Do the insns make any sense without TSX? Anyway - hence the
> comment, and if you're convinced the connection does not
> need making, I'd be okay dropping it. I would assume though
> that we'd better hide TSXLDTRK whenever we hide RTM, which
> is most easily achieved by having a connection here.

Actually - that is a very good point.  I expect there will (or should)
be an interaction with MSR_TSX_CTRL, as it has CPUID-hiding functionality.

For now, could I ask you to not expose this to guests in this patch?

For the emulator side of things alone I think this is ok (although
looking over it a second time, we could really do with a comment in the
code explaining that we're never in an RTM region, hence the nop behaviour).

I'll follow up with Intel, and we can figure out the CPUID derivation
details at a later point.

If you're happy with this plan, then A-by to save a round trip.

~Andrew

Re: [PATCH v2 2/3] various: Remove unnecessary OBJECT() cast

[Philippe Mathieu-Daudé]

On 5/8/20 2:49 PM, Markus Armbruster wrote:

Philippe Mathieu-Daudé  writes:


The OBJECT() macro is defined as:

   #define OBJECT(obj) ((Object *)(obj))

which expands to:

   ((Object *)object_dynamic_cast_assert((Object *)(obj), (name),
 __FILE__, __LINE__, __func__))


Nope :)


This assertion can only fail when @obj points to something other
than its stated type, i.e. when we're in undefined behavior country.


There is no assertion.


Remove the unnecessary OBJECT() casts when we already know the
pointer is of Object type.

Patch created mechanically using spatch with this script:

   @@
   typedef Object;
   Object *o;
   @@
   -   OBJECT(o)
   +   o

Acked-by: Cornelia Huck 
Acked-by: Corey Minyard 
Acked-by: John Snow 
Reviewed-by: Richard Henderson 
Signed-off-by: Philippe Mathieu-Daudé 
---
v2: Reword (Markus)


My rewording suggestion applied to PATCH 3, not to this one.


OK.



With v2's commit message;
Reviewed-by: Markus Armbruster 


Are you willing to take these patches? In that case, are you OK to take 
1 & 3 and I resend 2?


Thanks,

Phil.

Re: [PATCH v8 04/12] x86emul: support SERIALIZE

[Andrew Cooper]

On 08/05/2020 08:34, Jan Beulich wrote:
>>> @@ -5660,6 +5661,18 @@ x86_emulate(
>>>  goto done;
>>>  break;
>>>  
>>> +case 0xe8:
>>> +switch ( vex.pfx )
>>> +{
>>> +case vex_none: /* serialize */
>>> +host_and_vcpu_must_have(serialize);
>>> +asm volatile ( ".byte 0x0f, 0x01, 0xe8" );
>> There is very little need for an actual implementation here.  The VMExit
>> to get here is good enough.
>>
>> The only question is whether pre-unrestricted_guest Intel boxes are
>> liable to find this in real mode code.
> Apart from this we also shouldn't assume HVM in the core emulator,
> I think.

It's not assuming HVM.  Its just that there is no way we can end up
emulating this instruction from PV context.

If we could end up here in PV context, the exception causing us to
emulate to begin with would be good enough as well.

~Andrew

Re: Xen Coding style

[Tamas K Lengyel]

On Fri, May 8, 2020 at 6:21 AM Julien Grall  wrote:
>
> Hi Jan,
>
> On 08/05/2020 12:20, Jan Beulich wrote:
> > On 08.05.2020 12:00, Julien Grall wrote:
> >> You seem to be the maintainer with the most unwritten rules. Would
> >> you mind to have a try at writing a coding style based on it?
> >
> > On the basis that even small, single aspect patches to CODING_STYLE
> > have been ignored [1],
>
> Your thread is one of the example why I started this thread. Agreeing on
> specific rule doesn't work because it either result to bikesheding or
> there is not enough interest to review rule by rule.
>
> > I don't think this would be a good use of my
> > time.
>
> I would have assumed that the current situation (i.e
> nitpicking/bikeshedding on the ML) is not a good use of your time :).
>
> I would be happy to put some effort to help getting the coding style
> right, however I believe focusing on an overall coding style would value
> everyone's time better than a rule by rule discussion.
>
> > If I was promised (reasonable) feedback, I could take what I
> > have and try to add at least a few more things based on what I find
> > myself commenting on more frequently. But really I'd prefer it to
> > be done the other way around - for people to look at the patches
> > already sent, and for me to only subsequently send more. After all,
> > if already those adjustments are controversial, I don't think we
> > could settle on others.
> While I understand this requires another investment from your part, I am
> afraid it is going to be painful for someone else to go through all the
> existing coding style bikeshedding and infer your unwritten rules.
>
> It might be more beneficial for that person to pursue the work done by
> Tamas and Viktor in the past (see my previous e-mail). This may mean to
> adopt an existing coding style (BSD) and then tweak it.

Thanks Julien for restarting this discussion. IMHO agreeing on a set
of style rules ahead and then applying universally all at once is not
going to be productive since we are so all over the place. Instead, I
would recommend we start piece-by-piece. We introduce a baseline style
checker, then maintainers can decide when and if they want to move
their code-base to be under the automated style checker. That way we
have a baseline and each maintainer can decide on their own term when
they want to have their files be also style checked and in what form.
The upside of this route I think is pretty clear: we can have at least
partial automation even while we figure out what to do with some of
the more problematic files and quirks that are in our code-base. I
would highly prefer this route since I would immediately bring all
files I maintain over to the automated checker just so I never ever
have to deal with this again manually. What style is in use to me
really doesn't matter, BSD was very close with some minor tweaks, or
even what we use to check the style just as long as we have
_something_.

Cheers,
Tamas

Re: [PATCH] x86/PVH: PHYSDEVOP_pci_mmcfg_reserved should not blindly register a region

[Andrew Cooper]

On 08/05/2020 13:43, Jan Beulich wrote:
> The op has a register/unregister flag, and hence registration shouldn't
> happen unilaterally. Introduce unregister_vpci_mmcfg_handler() to handle
> this case.
>
> Fixes: eb3dd90e4089 ("x86/physdev: enable PHYSDEVOP_pci_mmcfg_reserved for 
> PVH Dom0")
> Signed-off-by: Jan Beulich 

I agree in principle that registration shouldn't be unilateral, but why
on earth does the API behave like that to begin with?

There is no provision to move or update MMCFG regions in any spec I'm
aware of, and hardware cannot in practice update memory routing like this.

Under what circumstances should we tolerate an unregister in the first
place?

~Andrew

Re: [PATCH v2 1/3] target: Remove unnecessary CPU() cast

[Markus Armbruster]

Philippe Mathieu-Daudé  writes:

> The CPU() macro is defined as:
>
>   #define CPU(obj) ((CPUState *)(obj))
>
> which expands to:
>
>   ((CPUState *)object_dynamic_cast_assert((Object *)(obj), (name),
>   __FILE__, __LINE__, __func__))
>
> This assertion can only fail when @obj points to something other
> than its stated type, i.e. when we're in undefined behavior country.
>
> Remove the unnecessary CPU() casts when we already know the pointer
> is of CPUState type.
>
> Patch created mechanically using spatch with this script:
>
>   @@
>   typedef CPUState;
>   CPUState *s;
>   @@
>   -   CPU(s)
>   +   s
>
> Acked-by: David Gibson 
> Reviewed-by: Cédric Le Goater 
> Reviewed-by: Richard Henderson 
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Markus Armbruster 

Re: [PATCH v2 2/3] various: Remove unnecessary OBJECT() cast

[Markus Armbruster]

Philippe Mathieu-Daudé  writes:

> The OBJECT() macro is defined as:
>
>   #define OBJECT(obj) ((Object *)(obj))
>
> which expands to:
>
>   ((Object *)object_dynamic_cast_assert((Object *)(obj), (name),
> __FILE__, __LINE__, __func__))

Nope :)

> This assertion can only fail when @obj points to something other
> than its stated type, i.e. when we're in undefined behavior country.

There is no assertion.

> Remove the unnecessary OBJECT() casts when we already know the
> pointer is of Object type.
>
> Patch created mechanically using spatch with this script:
>
>   @@
>   typedef Object;
>   Object *o;
>   @@
>   -   OBJECT(o)
>   +   o
>
> Acked-by: Cornelia Huck 
> Acked-by: Corey Minyard 
> Acked-by: John Snow 
> Reviewed-by: Richard Henderson 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> v2: Reword (Markus)

My rewording suggestion applied to PATCH 3, not to this one.

With v2's commit message;
Reviewed-by: Markus Armbruster 

Re: [PATCH v2 3/3] hw: Remove unnecessary DEVICE() cast

[Markus Armbruster]

Philippe Mathieu-Daudé  writes:

> The DEVICE() macro is defined as:
>
>   #define DEVICE(obj) OBJECT_CHECK(DeviceState, (obj), TYPE_DEVICE)
>
> which expands to:
>
>   ((DeviceState *)object_dynamic_cast_assert((Object *)(obj), (name),
>  __FILE__, __LINE__,
>  __func__))
>
> This assertion can only fail when @obj points to something other
> than its stated type, i.e. when we're in undefined behavior country.
>
> Remove the unnecessary DEVICE() casts when we already know the
> pointer is of DeviceState type.
>
> Patch created mechanically using spatch with this script:
>
>   @@
>   typedef DeviceState;
>   DeviceState *s;
>   @@
>   -   DEVICE(s)
>   +   s
>
> Acked-by: David Gibson 
> Acked-by: Paul Durrant 
> Reviewed-by: Markus Armbruster 
> Reviewed-by: Cédric Le Goater 
> Acked-by: John Snow 
> Reviewed-by: Richard Henderson 
> Signed-off-by: Philippe Mathieu-Daudé 

Lovely.

Reviewed-by: Markus Armbruster 

Now repeat this exercise for each QOM type cast macro :)

[PATCH] x86/PVH: PHYSDEVOP_pci_mmcfg_reserved should not blindly register a region

[Jan Beulich]

The op has a register/unregister flag, and hence registration shouldn't
happen unilaterally. Introduce unregister_vpci_mmcfg_handler() to handle
this case.

Fixes: eb3dd90e4089 ("x86/physdev: enable PHYSDEVOP_pci_mmcfg_reserved for PVH 
Dom0")
Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/hvm/io.c
+++ b/xen/arch/x86/hvm/io.c
@@ -558,6 +558,47 @@ int register_vpci_mmcfg_handler(struct d
 return 0;
 }
 
+int unregister_vpci_mmcfg_handler(struct domain *d, paddr_t addr,
+  unsigned int start_bus, unsigned int end_bus,
+  unsigned int seg)
+{
+struct hvm_mmcfg *mmcfg;
+int rc = -ENOENT;
+
+ASSERT(is_hardware_domain(d));
+
+if ( start_bus > end_bus )
+return -EINVAL;
+
+write_lock(>arch.hvm.mmcfg_lock);
+
+list_for_each_entry ( mmcfg, >arch.hvm.mmcfg_regions, next )
+if ( mmcfg->addr == addr + (start_bus << 20) &&
+ mmcfg->segment == seg &&
+ mmcfg->start_bus == start_bus &&
+ mmcfg->size == ((end_bus - start_bus + 1) << 20) )
+{
+list_del(>next);
+if ( !list_empty(>arch.hvm.mmcfg_regions) )
+xfree(mmcfg);
+else
+{
+/*
+ * Cannot unregister the MMIO handler - leave a fake entry
+ * on the list.
+ */
+memset(mmcfg, 0, sizeof(*mmcfg));
+list_add(>next, >arch.hvm.mmcfg_regions);
+}
+rc = 0;
+break;
+}
+
+write_unlock(>arch.hvm.mmcfg_lock);
+
+return rc;
+}
+
 void destroy_vpci_mmcfg(struct domain *d)
 {
 struct list_head *mmcfg_regions = >arch.hvm.mmcfg_regions;
--- a/xen/arch/x86/physdev.c
+++ b/xen/arch/x86/physdev.c
@@ -559,12 +559,18 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H
 if ( !ret && has_vpci(currd) )
 {
 /*
- * For HVM (PVH) domains try to add the newly found MMCFG to the
- * domain.
+ * For HVM (PVH) domains try to add/remove the reported MMCFG
+ * to/from the domain.
  */
-ret = register_vpci_mmcfg_handler(currd, info.address,
-  info.start_bus, info.end_bus,
-  info.segment);
+if ( info.flags & XEN_PCI_MMCFG_RESERVED )
+ret = register_vpci_mmcfg_handler(currd, info.address,
+  info.start_bus, info.end_bus,
+  info.segment);
+else
+ret = unregister_vpci_mmcfg_handler(currd, info.address,
+info.start_bus,
+info.end_bus,
+info.segment);
 }
 
 break;
--- a/xen/include/asm-x86/hvm/io.h
+++ b/xen/include/asm-x86/hvm/io.h
@@ -178,6 +178,9 @@ void register_vpci_portio_handler(struct
 int register_vpci_mmcfg_handler(struct domain *d, paddr_t addr,
 unsigned int start_bus, unsigned int end_bus,
 unsigned int seg);
+int unregister_vpci_mmcfg_handler(struct domain *d, paddr_t addr,
+  unsigned int start_bus, unsigned int end_bus,
+  unsigned int seg);
 /* Destroy tracked MMCFG areas. */
 void destroy_vpci_mmcfg(struct domain *d);
 

Re: Xen Coding style

[Julien Grall]

Hi Jan,

On 08/05/2020 12:20, Jan Beulich wrote:

On 08.05.2020 12:00, Julien Grall wrote:

You seem to be the maintainer with the most unwritten rules. Would
you mind to have a try at writing a coding style based on it?


On the basis that even small, single aspect patches to CODING_STYLE
have been ignored [1],


Your thread is one of the example why I started this thread. Agreeing on 
specific rule doesn't work because it either result to bikesheding or 
there is not enough interest to review rule by rule.



I don't think this would be a good use of my
time.


I would have assumed that the current situation (i.e 
nitpicking/bikeshedding on the ML) is not a good use of your time :).


I would be happy to put some effort to help getting the coding style 
right, however I believe focusing on an overall coding style would value 
everyone's time better than a rule by rule discussion.



If I was promised (reasonable) feedback, I could take what I
have and try to add at least a few more things based on what I find
myself commenting on more frequently. But really I'd prefer it to
be done the other way around - for people to look at the patches
already sent, and for me to only subsequently send more. After all,
if already those adjustments are controversial, I don't think we
could settle on others.
While I understand this requires another investment from your part, I am 
afraid it is going to be painful for someone else to go through all the 
existing coding style bikeshedding and infer your unwritten rules.


It might be more beneficial for that person to pursue the work done by 
Tamas and Viktor in the past (see my previous e-mail). This may mean to 
adopt an existing coding style (BSD) and then tweak it.


Cheers,

--
Julien Grall

[xen-unstable-smoke test] 150088: tolerable all pass - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  e0d92d9bd7997c6bcda17a19aba4f3957dd1a2e9
baseline version:
 xen  35b819c45c4603fdb1d400925d6b2e6f8689a9d5

Last test of basis   150080  2020-05-07 19:01:35 Z0 days
Testing same since   150088  2020-05-08 09:00:36 Z0 days1 attempts


People who touched revisions under test:
  Dario Faggioli 
  Juergen Gross 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-amd64-libvirt pass



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   35b819c45c..e0d92d9bd7  e0d92d9bd7997c6bcda17a19aba4f3957dd1a2e9 -> smoke

[xen-4.13-testing test] 150073: tolerable FAIL - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

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

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail  like 150042
 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-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-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-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-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-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 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-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-amd64-i386-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-i386-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-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-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-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-win7-amd64 17 guest-stop fail never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-amd64-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-ws16-amd64 17 guest-stop fail never pass

version targeted for testing:
 xen  9649b83b2ab4707de79da42307f8757e317bf217
baseline version:
 xen  d2aecd86c4481291b260869c47cf0a9a02321564

Last test of basis   150042  2020-05-05 16:06:37 Z2 days
Testing same since   150073  2020-05-07 13:06:17 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Ashok Raj 
  Borislav Petkov 
  Hongyan Xia 
  Jan 

Re: Xen Coding style

[Jan Beulich]

On 08.05.2020 12:00, Julien Grall wrote:
> You seem to be the maintainer with the most unwritten rules. Would
> you mind to have a try at writing a coding style based on it?

On the basis that even small, single aspect patches to CODING_STYLE
have been ignored [1], I don't think this would be a good use of my
time. If I was promised (reasonable) feedback, I could take what I
have and try to add at least a few more things based on what I find
myself commenting on more frequently. But really I'd prefer it to
be done the other way around - for people to look at the patches
already sent, and for me to only subsequently send more. After all,
if already those adjustments are controversial, I don't think we
could settle on others.

Jan

[1] https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg01234.html

Re: [PATCH v3] accel: Move Xen accelerator code under accel/xen/

[Juan Quintela]

Philippe Mathieu-Daudé  wrote:
> This code is not related to hardware emulation.
> Move it under accel/ with the other hypervisors.
>
> Reviewed-by: Paul Durrant 
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Juan Quintela 

[PATCH v3] accel: Move Xen accelerator code under accel/xen/

[Philippe Mathieu-Daudé]

This code is not related to hardware emulation.
Move it under accel/ with the other hypervisors.

Reviewed-by: Paul Durrant 
Signed-off-by: Philippe Mathieu-Daudé 
---
We could also move the memory management functions from
hw/i386/xen/xen-hvm.c but it is not trivial.

v2: Use g_assert_not_reached() instead of abort()
v3: (quintela)
 - Do not expose xen_allowed
 - Do not abort in xen_hvm_modified_memory
---
 include/exec/ram_addr.h|  2 +-
 include/hw/xen/xen.h   | 11 ---
 include/sysemu/xen.h   | 38 ++
 hw/xen/xen-common.c => accel/xen/xen-all.c |  8 +
 hw/acpi/piix4.c|  2 +-
 hw/i386/pc.c   |  1 +
 hw/i386/pc_piix.c  |  1 +
 hw/i386/pc_q35.c   |  1 +
 hw/i386/xen/xen-hvm.c  |  1 +
 hw/i386/xen/xen_platform.c |  1 +
 hw/isa/piix3.c |  1 +
 hw/pci/msix.c  |  1 +
 migration/savevm.c |  2 +-
 softmmu/vl.c   |  2 +-
 stubs/xen-hvm.c|  9 -
 target/i386/cpu.c  |  2 +-
 MAINTAINERS|  2 ++
 accel/Makefile.objs|  1 +
 accel/xen/Makefile.objs|  1 +
 hw/xen/Makefile.objs   |  2 +-
 20 files changed, 63 insertions(+), 26 deletions(-)
 create mode 100644 include/sysemu/xen.h
 rename hw/xen/xen-common.c => accel/xen/xen-all.c (98%)
 create mode 100644 accel/xen/Makefile.objs

diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 5e59a3d8d7..4e05292f91 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -21,7 +21,7 @@
 
 #ifndef CONFIG_USER_ONLY
 #include "cpu.h"
-#include "hw/xen/xen.h"
+#include "sysemu/xen.h"
 #include "sysemu/tcg.h"
 #include "exec/ramlist.h"
 #include "exec/ramblock.h"
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index 5ac1c6dc55..771dd447f2 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -20,13 +20,6 @@ extern uint32_t xen_domid;
 extern enum xen_mode xen_mode;
 extern bool xen_domid_restrict;
 
-extern bool xen_allowed;
-
-static inline bool xen_enabled(void)
-{
-return xen_allowed;
-}
-
 int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num);
 void xen_piix3_set_irq(void *opaque, int irq_num, int level);
 void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len);
@@ -39,10 +32,6 @@ void xenstore_store_pv_console_info(int i, struct Chardev 
*chr);
 
 void xen_hvm_init(PCMachineState *pcms, MemoryRegion **ram_memory);
 
-void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
-   struct MemoryRegion *mr, Error **errp);
-void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length);
-
 void xen_register_framebuffer(struct MemoryRegion *mr);
 
 #endif /* QEMU_HW_XEN_H */
diff --git a/include/sysemu/xen.h b/include/sysemu/xen.h
new file mode 100644
index 00..1ca292715e
--- /dev/null
+++ b/include/sysemu/xen.h
@@ -0,0 +1,38 @@
+/*
+ * QEMU Xen support
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef SYSEMU_XEN_H
+#define SYSEMU_XEN_H
+
+#ifdef CONFIG_XEN
+
+bool xen_enabled(void);
+
+#ifndef CONFIG_USER_ONLY
+void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length);
+void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
+   struct MemoryRegion *mr, Error **errp);
+#endif
+
+#else /* !CONFIG_XEN */
+
+#define xen_enabled() 0
+#ifndef CONFIG_USER_ONLY
+static inline void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
+{
+/* nothing */
+}
+static inline void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
+ MemoryRegion *mr, Error **errp)
+{
+g_assert_not_reached();
+}
+#endif
+
+#endif /* CONFIG_XEN */
+
+#endif
diff --git a/hw/xen/xen-common.c b/accel/xen/xen-all.c
similarity index 98%
rename from hw/xen/xen-common.c
rename to accel/xen/xen-all.c
index a15070f7f6..4f22c53731 100644
--- a/hw/xen/xen-common.c
+++ b/accel/xen/xen-all.c
@@ -16,6 +16,7 @@
 #include "hw/xen/xen_pt.h"
 #include "chardev/char.h"
 #include "sysemu/accel.h"
+#include "sysemu/xen.h"
 #include "sysemu/runstate.h"
 #include "migration/misc.h"
 #include "migration/global_state.h"
@@ -31,6 +32,13 @@
 do { } while (0)
 #endif
 
+static bool xen_allowed;
+
+bool xen_enabled(void)
+{
+return xen_allowed;
+}
+
 xc_interface *xen_xc;
 xenforeignmemory_handle *xen_fmem;
 xendevicemodel_handle *xen_dmod;
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 964d6f5990..daed273687 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -30,6 +30,7 @@
 #include "hw/acpi/acpi.h"
 #include "sysemu/runstate.h"
 #include "sysemu/sysemu.h"
+#include 

Re: Xen Coding style

[Julien Grall]

On 08/05/2020 09:36, Jan Beulich wrote:

On 07.05.2020 16:43, Julien Grall wrote:

This was originally discussed during the last community call.

A major part of the comments during review is related to coding style issues. 
This can lead to frustration from the contributor as the current CODING_STYLE 
is quite light and the choice often down to a matter of taste from the 
maintainers.

In the past, Lars tried to address the problem by introducing a coding style 
checker (see [1] and [2]). However, the work came to stop because we couldn't 
agree on what is Xen coding style.

I would like to suggest a different approach. Rather than trying to agree on 
all the rules one by one, I propose to have a vote on different coding styles 
and pick the preferred one.

The list of coding styles would come from the community. It could be coding 
styles already used in the open source community or new coding styles (for 
instance a contributor could write down his/her understanding of Xen Coding 
Style).

Are the committers happy with this appraoch?


I'm not, sorry, and I'm pretty sure I indicated so before. For one I
don't think picking an arbitrary other style than what we currently use
is going to be helpful: It'll be a significant amount of code churn all
over the code base. Instead I think the basic current principle should
remain: Imported files, if not heavily changed, are permitted to retain
their original style, while "our" files get written in "our" style.
(Recording which one's which is still tbd.)


There are existing coding styles that are quite close to the one used by 
Xen (such as BSD). We could either use them as-is or make small changes 
to fit Xen.




I don't think though this necessarily implies "to agree on all the rules
one by one" - we could also settle on there explicitly being freedom
beyond what's spelled out explicitly. I'd not be happy with this, as it
would lead to a lot of inconsistencies over time, but it's still an
option. Obviously there's the wide range of middle ground to agree on
some more rules to become written down (I did propose a few over time,
without - iirc - getting helpful, if any, feedback), while leaving the
rest up to the author.

The main thing we need to settle on imo is whether unwritten rules
count. Me being picky isn't because of me liking to be, but because of
me thinking that a consistent code base is quite helpful. If consensus
is that consistency is not a goal, I'll accept this (with some
grumbling).


Consistency is important, but this should not be based on unwritten 
rules. We should be able to write a script that can check whether a 
patch contain any violations.


The end goal is to reduce the workload on the reviewer and the tension 
within the community.


You seem to be the maintainer with the most unwritten rules. Would you 
mind to have a try at writing a coding style based on it?


Cheers,

--
Julien Grall

Re: [PATCH v2 1/2] exec: Check Xen is enabled before calling the Xen API

[Philippe Mathieu-Daudé]

Hi Juan,

On 5/8/20 10:39 AM, Juan Quintela wrote:

Philippe Mathieu-Daudé  wrote:

Signed-off-by: Philippe Mathieu-Daudé 
---
  include/exec/ram_addr.h | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 5e59a3d8d7..dd8713179e 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -330,7 +330,9 @@ static inline void 
cpu_physical_memory_set_dirty_range(ram_addr_t start,
  }
  }
  
-xen_hvm_modified_memory(start, length);

+if (xen_enabled()) {
+xen_hvm_modified_memory(start, length);
+}
  }
  
  #if !defined(_WIN32)

@@ -388,7 +390,9 @@ static inline void 
cpu_physical_memory_set_dirty_lebitmap(unsigned long *bitmap,
  }
  }
  
-xen_hvm_modified_memory(start, pages << TARGET_PAGE_BITS);

+if (xen_enabled()) {
+xen_hvm_modified_memory(start, pages << TARGET_PAGE_BITS);
+}
  } else {
  uint8_t clients = tcg_enabled() ? DIRTY_CLIENTS_ALL : 
DIRTY_CLIENTS_NOCODE;


I don't object moving the xen code to accell.  But I think that this
change is bad.

On the following patch:
- You export xen_allowed
   (ok, it was already exported, but I think it shouldn't)

(master)$ find . -type f | xargs grep xen_allowed
./hw/xen/xen-common.c:ac->allowed = _allowed;
./include/hw/xen/xen.h:extern bool xen_allowed;
./include/hw/xen/xen.h:return xen_allowed;
./softmmu/vl.c:bool xen_allowed;

This are all the users that I can find.

And xen_havm_modified_memory() is an empty function if xen is not
compiled in.  And in the case that xen is compiled in, the 1st thing
that it checks is:

if (unlikely(xen_in_migration)) {

That is way more restrictive that xen_enabled().

So, I think that it is better to drop this patch, maintain next one, but
just un-exporting xen_allowed.

What do you think?


I blindly trust your judgement on this :) I'd rather not touch this code 
but as it happens to be in "exec/ram_addr.h" I had to modify it.


Thanks for your reviews!



Later, Juan.

Re: [PATCH v2 1/2] exec: Check Xen is enabled before calling the Xen API

[Juan Quintela]

Philippe Mathieu-Daudé  wrote:
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  include/exec/ram_addr.h | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> index 5e59a3d8d7..dd8713179e 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -330,7 +330,9 @@ static inline void 
> cpu_physical_memory_set_dirty_range(ram_addr_t start,
>  }
>  }
>  
> -xen_hvm_modified_memory(start, length);
> +if (xen_enabled()) {
> +xen_hvm_modified_memory(start, length);
> +}
>  }
>  
>  #if !defined(_WIN32)
> @@ -388,7 +390,9 @@ static inline void 
> cpu_physical_memory_set_dirty_lebitmap(unsigned long *bitmap,
>  }
>  }
>  
> -xen_hvm_modified_memory(start, pages << TARGET_PAGE_BITS);
> +if (xen_enabled()) {
> +xen_hvm_modified_memory(start, pages << TARGET_PAGE_BITS);
> +}
>  } else {
>  uint8_t clients = tcg_enabled() ? DIRTY_CLIENTS_ALL : 
> DIRTY_CLIENTS_NOCODE;

I don't object moving the xen code to accell.  But I think that this
change is bad.

On the following patch:
- You export xen_allowed
  (ok, it was already exported, but I think it shouldn't)

(master)$ find . -type f | xargs grep xen_allowed
./hw/xen/xen-common.c:ac->allowed = _allowed;
./include/hw/xen/xen.h:extern bool xen_allowed;
./include/hw/xen/xen.h:return xen_allowed;
./softmmu/vl.c:bool xen_allowed;

This are all the users that I can find.

And xen_havm_modified_memory() is an empty function if xen is not
compiled in.  And in the case that xen is compiled in, the 1st thing
that it checks is:

   if (unlikely(xen_in_migration)) {

That is way more restrictive that xen_enabled().

So, I think that it is better to drop this patch, maintain next one, but
just un-exporting xen_allowed.

What do you think?

Later, Juan.

Re: Xen Coding style

[Jan Beulich]

On 07.05.2020 16:43, Julien Grall wrote:
> This was originally discussed during the last community call.
> 
> A major part of the comments during review is related to coding style issues. 
> This can lead to frustration from the contributor as the current CODING_STYLE 
> is quite light and the choice often down to a matter of taste from the 
> maintainers.
> 
> In the past, Lars tried to address the problem by introducing a coding style 
> checker (see [1] and [2]). However, the work came to stop because we couldn't 
> agree on what is Xen coding style.
> 
> I would like to suggest a different approach. Rather than trying to agree on 
> all the rules one by one, I propose to have a vote on different coding styles 
> and pick the preferred one.
> 
> The list of coding styles would come from the community. It could be coding 
> styles already used in the open source community or new coding styles (for 
> instance a contributor could write down his/her understanding of Xen Coding 
> Style).
> 
> Are the committers happy with this appraoch?

I'm not, sorry, and I'm pretty sure I indicated so before. For one I
don't think picking an arbitrary other style than what we currently use
is going to be helpful: It'll be a significant amount of code churn all
over the code base. Instead I think the basic current principle should
remain: Imported files, if not heavily changed, are permitted to retain
their original style, while "our" files get written in "our" style.
(Recording which one's which is still tbd.)

I don't think though this necessarily implies "to agree on all the rules
one by one" - we could also settle on there explicitly being freedom
beyond what's spelled out explicitly. I'd not be happy with this, as it
would lead to a lot of inconsistencies over time, but it's still an
option. Obviously there's the wide range of middle ground to agree on
some more rules to become written down (I did propose a few over time,
without - iirc - getting helpful, if any, feedback), while leaving the
rest up to the author.

The main thing we need to settle on imo is whether unwritten rules
count. Me being picky isn't because of me liking to be, but because of
me thinking that a consistent code base is quite helpful. If consensus
is that consistency is not a goal, I'll accept this (with some
grumbling).

Jan

Re: [PATCH 3/3] xen/cpupool: fix removing cpu from a cpupool

[Jürgen Groß]

On 08.05.20 10:19, Jan Beulich wrote:

On 07.05.2020 20:36, Dario Faggioli wrote:

On Thu, 2020-04-30 at 17:15 +0200, Juergen Gross wrote:

Commit cb563d7665f2 ("xen/sched: support core scheduling for moving
cpus to/from cpupools") introduced a regression when trying to remove
an offline cpu from a cpupool, as the system would crash in this
situation.

Fix that by testing the cpu to be online.

Fixes: cb563d7665f2 ("xen/sched: support core scheduling for moving
cpus to/from cpupools")
Signed-off-by: Juergen Gross 


Acked-by: Dario Faggioli 


Jürgen,

it looks like this is independent of the earlier two patches
and hence could go in, while I understand there'll be v2 for
the earlier ones. Please confirm.


Confirmed.


Juergen

Re: [PATCH 3/3] xen/cpupool: fix removing cpu from a cpupool

[Jan Beulich]

On 07.05.2020 20:36, Dario Faggioli wrote:
> On Thu, 2020-04-30 at 17:15 +0200, Juergen Gross wrote:
>> Commit cb563d7665f2 ("xen/sched: support core scheduling for moving
>> cpus to/from cpupools") introduced a regression when trying to remove
>> an offline cpu from a cpupool, as the system would crash in this
>> situation.
>>
>> Fix that by testing the cpu to be online.
>>
>> Fixes: cb563d7665f2 ("xen/sched: support core scheduling for moving
>> cpus to/from cpupools")
>> Signed-off-by: Juergen Gross 
>>
> Acked-by: Dario Faggioli 

Jürgen,

it looks like this is independent of the earlier two patches
and hence could go in, while I understand there'll be v2 for
the earlier ones. Please confirm.

Jan

Re: [PATCH v8 01/12] x86emul: disable FPU/MMX/SIMD insn emulation when !HVM

[Jan Beulich]

On 07.05.2020 20:11, Andrew Cooper wrote:
> On 05/05/2020 09:12, Jan Beulich wrote:
>> In a pure PV environment (the PV shim in particular) we don't really
>> need emulation of all these. To limit #ifdef-ary utilize some of the
>> CASE_*() macros we have, by providing variants expanding to
>> (effectively) nothing (really a label, which in turn requires passing
>> -Wno-unused-label to the compiler when build such configurations).
>>
>> Due to the mixture of macro and #ifdef use, the placement of some of
>> the #ifdef-s is a little arbitrary.
>>
>> The resulting object file's .text is less than half the size of the
>> original, and looks to also be compiling a little more quickly.
>>
>> This is meant as a first step; more parts can likely be disabled down
>> the road.
>>
>> Suggested-by: Andrew Cooper 
>> Signed-off-by: Jan Beulich 
>> ---
>> v7: Integrate into this series. Re-base.
>> ---
>> I'll be happy to take suggestions allowing to avoid -Wno-unused-label.
> 
> I already gave you one, along with a far less invasive change.
> 
> It is not interesting to be able to conditionally compile these things
> separately.  A build of Xen will either need everything, or just the
> integer group.

And I had replied to that, indicating my (at least partial)
disagreement as well as asking for clarification on an apparently
incomplete sentence in your response.

Note that in an initial (3 months earlier) reply you did say

"On that subject, it would be very helpful to at least be able to
 configure reduced builds from these utilities."

which I responded to

"Yes, I too have been thinking this way. I may get there eventually."

I specifically meant FPU-less, MMX-less, and SIMD-less each on their
own.

I'm also not at all convinced of, as you say, "a build of Xen will
either need everything, or just the integer group". Yes, for now I
unilaterally disable all three for !HVM here, but that's just
because we know of no PV guests trying to update their page tables
in "interesting" ways. Long term I think this would better be a
separate Kconfig option (or multiple ones, if we stick to keeping
the three groups here to have separate controls), merely defaulting
to !HVM. I could of course switch to such an approach right away.

Jan

[PATCH v2 2/2] accel: Move Xen accelerator code under accel/xen/

[Philippe Mathieu-Daudé]

This code is not related to hardware emulation.
Move it under accel/ with the other hypervisors.

Reviewed-by: Paul Durrant 
Signed-off-by: Philippe Mathieu-Daudé 
---
We could also move the memory management functions from
hw/i386/xen/xen-hvm.c but it is not trivial.

v2: Use g_assert_not_reached() instead of abort()
---
 include/exec/ram_addr.h|  2 +-
 include/hw/xen/xen.h   | 11 --
 include/sysemu/xen.h   | 40 ++
 hw/xen/xen-common.c => accel/xen/xen-all.c |  3 ++
 hw/acpi/piix4.c|  2 +-
 hw/i386/pc.c   |  1 +
 hw/i386/pc_piix.c  |  1 +
 hw/i386/pc_q35.c   |  1 +
 hw/i386/xen/xen-hvm.c  |  1 +
 hw/i386/xen/xen_platform.c |  1 +
 hw/isa/piix3.c |  1 +
 hw/pci/msix.c  |  1 +
 migration/savevm.c |  2 +-
 softmmu/vl.c   |  2 +-
 stubs/xen-hvm.c|  9 -
 target/i386/cpu.c  |  2 +-
 MAINTAINERS|  2 ++
 accel/Makefile.objs|  1 +
 accel/xen/Makefile.objs|  1 +
 hw/xen/Makefile.objs   |  2 +-
 20 files changed, 60 insertions(+), 26 deletions(-)
 create mode 100644 include/sysemu/xen.h
 rename hw/xen/xen-common.c => accel/xen/xen-all.c (99%)
 create mode 100644 accel/xen/Makefile.objs

diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index dd8713179e..a94809f89b 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -21,7 +21,7 @@
 
 #ifndef CONFIG_USER_ONLY
 #include "cpu.h"
-#include "hw/xen/xen.h"
+#include "sysemu/xen.h"
 #include "sysemu/tcg.h"
 #include "exec/ramlist.h"
 #include "exec/ramblock.h"
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index 5ac1c6dc55..771dd447f2 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -20,13 +20,6 @@ extern uint32_t xen_domid;
 extern enum xen_mode xen_mode;
 extern bool xen_domid_restrict;
 
-extern bool xen_allowed;
-
-static inline bool xen_enabled(void)
-{
-return xen_allowed;
-}
-
 int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num);
 void xen_piix3_set_irq(void *opaque, int irq_num, int level);
 void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len);
@@ -39,10 +32,6 @@ void xenstore_store_pv_console_info(int i, struct Chardev 
*chr);
 
 void xen_hvm_init(PCMachineState *pcms, MemoryRegion **ram_memory);
 
-void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
-   struct MemoryRegion *mr, Error **errp);
-void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length);
-
 void xen_register_framebuffer(struct MemoryRegion *mr);
 
 #endif /* QEMU_HW_XEN_H */
diff --git a/include/sysemu/xen.h b/include/sysemu/xen.h
new file mode 100644
index 00..e3d30e327f
--- /dev/null
+++ b/include/sysemu/xen.h
@@ -0,0 +1,40 @@
+/*
+ * QEMU Xen support
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef SYSEMU_XEN_H
+#define SYSEMU_XEN_H
+
+#ifdef CONFIG_XEN
+
+extern bool xen_allowed;
+
+#define xen_enabled() (xen_allowed)
+
+#ifndef CONFIG_USER_ONLY
+void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length);
+void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
+   struct MemoryRegion *mr, Error **errp);
+#endif
+
+#else /* !CONFIG_XEN */
+
+#define xen_enabled() 0
+#ifndef CONFIG_USER_ONLY
+static inline void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
+{
+g_assert_not_reached();
+}
+static inline void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
+ MemoryRegion *mr, Error **errp)
+{
+g_assert_not_reached();
+}
+#endif
+
+#endif /* CONFIG_XEN */
+
+#endif
diff --git a/hw/xen/xen-common.c b/accel/xen/xen-all.c
similarity index 99%
rename from hw/xen/xen-common.c
rename to accel/xen/xen-all.c
index a15070f7f6..5c64571cb8 100644
--- a/hw/xen/xen-common.c
+++ b/accel/xen/xen-all.c
@@ -16,6 +16,7 @@
 #include "hw/xen/xen_pt.h"
 #include "chardev/char.h"
 #include "sysemu/accel.h"
+#include "sysemu/xen.h"
 #include "sysemu/runstate.h"
 #include "migration/misc.h"
 #include "migration/global_state.h"
@@ -31,6 +32,8 @@
 do { } while (0)
 #endif
 
+bool xen_allowed;
+
 xc_interface *xen_xc;
 xenforeignmemory_handle *xen_fmem;
 xendevicemodel_handle *xen_dmod;
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 964d6f5990..daed273687 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -30,6 +30,7 @@
 #include "hw/acpi/acpi.h"
 #include "sysemu/runstate.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/xen.h"
 #include "qapi/error.h"
 #include "qemu/range.h"
 #include "exec/address-spaces.h"
@@ -41,7 +42,6 @@
 

[PATCH v2 1/2] exec: Check Xen is enabled before calling the Xen API

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/exec/ram_addr.h | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 5e59a3d8d7..dd8713179e 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -330,7 +330,9 @@ static inline void 
cpu_physical_memory_set_dirty_range(ram_addr_t start,
 }
 }
 
-xen_hvm_modified_memory(start, length);
+if (xen_enabled()) {
+xen_hvm_modified_memory(start, length);
+}
 }
 
 #if !defined(_WIN32)
@@ -388,7 +390,9 @@ static inline void 
cpu_physical_memory_set_dirty_lebitmap(unsigned long *bitmap,
 }
 }
 
-xen_hvm_modified_memory(start, pages << TARGET_PAGE_BITS);
+if (xen_enabled()) {
+xen_hvm_modified_memory(start, pages << TARGET_PAGE_BITS);
+}
 } else {
 uint8_t clients = tcg_enabled() ? DIRTY_CLIENTS_ALL : 
DIRTY_CLIENTS_NOCODE;
 
-- 
2.21.3

[PATCH v2 0/2] accel: Move Xen accelerator code under accel/xen/

[Philippe Mathieu-Daudé]

Supersedes: <20200507155813.16169-1-phi...@redhat.com>

Philippe Mathieu-Daudé (2):
  exec: Check Xen is enabled before calling the Xen API
  accel: Move Xen accelerator code under accel/xen/

 include/exec/ram_addr.h| 10 --
 include/hw/xen/xen.h   | 11 --
 include/sysemu/xen.h   | 40 ++
 hw/xen/xen-common.c => accel/xen/xen-all.c |  3 ++
 hw/acpi/piix4.c|  2 +-
 hw/i386/pc.c   |  1 +
 hw/i386/pc_piix.c  |  1 +
 hw/i386/pc_q35.c   |  1 +
 hw/i386/xen/xen-hvm.c  |  1 +
 hw/i386/xen/xen_platform.c |  1 +
 hw/isa/piix3.c |  1 +
 hw/pci/msix.c  |  1 +
 migration/savevm.c |  2 +-
 softmmu/vl.c   |  2 +-
 stubs/xen-hvm.c|  9 -
 target/i386/cpu.c  |  2 +-
 MAINTAINERS|  2 ++
 accel/Makefile.objs|  1 +
 accel/xen/Makefile.objs|  1 +
 hw/xen/Makefile.objs   |  2 +-
 20 files changed, 66 insertions(+), 28 deletions(-)
 create mode 100644 include/sysemu/xen.h
 rename hw/xen/xen-common.c => accel/xen/xen-all.c (99%)
 create mode 100644 accel/xen/Makefile.objs

-- 
2.21.3

Re: [PATCH v2 1/4] x86/mm: no-one passes a NULL domain to init_xen_l4_slots()

[Jan Beulich]

On 07.05.2020 19:26, Andrew Cooper wrote:
> On 05/05/2020 07:31, Jan Beulich wrote:
>> On 21.04.2020 18:40, Roger Pau Monné wrote:
>>> On Tue, Apr 21, 2020 at 11:11:03AM +0200, Jan Beulich wrote:
 Drop the NULL checks - they've been introduced by commit 8d7b633ada
 ("x86/mm: Consolidate all Xen L4 slot writing into
 init_xen_l4_slots()") for no apparent reason.

 Signed-off-by: Jan Beulich 
>>> Reviewed-by: Roger Pau Monné 
>> you weren't entirely happy with the change because of the
>> possible (or, as you state, necessary) need to undo this. I
>> still think in the current shape the NULL checks are
>> pointless and hence would better go away. Re-introducing them
>> (adjusted to whatever shape the function may be in by that
>> time) is not that big of a problem. May I ask that you
>> explicitly clarify whether you actively NAK the patch, accept
>> it going in with Roger's R-b, or would be willing to ack it?
> 
> I'm not going to nack it, because that would be petty, but I still don't
> think it is a useful use of your time to be making more work for someone
> in the future to revert.
> 
> However, if you wish to take the patch with Roger's R-b, then please fix
> the stale commit message, seeing as this is v2 and I explained exactly
> why it was done like that.

Is "... without giving a reason; I'm told this was done in anticipation
of the function potentially getting called with a NULL argument" any
better? I don't think the commit message here was stale, as said commit
indeed gives no explanation, yet all call sites pass non-NULL.

Jan

Re: [PATCH v8 05/12] x86emul: support X{SUS,RES}LDTRK

[Jan Beulich]

On 07.05.2020 22:13, Andrew Cooper wrote:
> On 05/05/2020 09:14, Jan Beulich wrote:
>> --- a/xen/tools/gen-cpuid.py
>> +++ b/xen/tools/gen-cpuid.py
>> @@ -284,6 +284,9 @@ def crunch_numbers(state):
>>  # as dependent features simplifies Xen's logic, and prevents the 
>> guest
>>  # from seeing implausible configurations.
>>  IBRSB: [STIBP, SSBD],
>> +
>> +# In principle the TSXLDTRK insns could also be considered 
>> independent.
>> +RTM: [TSXLDTRK],
> 
> Why the link?  There is no relevant interaction AFAICT.

Do the insns make any sense without TSX? Anyway - hence the
comment, and if you're convinced the connection does not
need making, I'd be okay dropping it. I would assume though
that we'd better hide TSXLDTRK whenever we hide RTM, which
is most easily achieved by having a connection here.

Jan

Re: [bug report] drm/xen-front: Add support for Xen PV display frontend

[Oleksandr Andrushchenko]

On 4/21/20 2:51 PM, Dan Carpenter wrote:
> It turns out there aren't that many of these in xen.
>
> $ grep IS_ERR_OR_NULL drivers/gpu/drm/xen/ -Rn
> drivers/gpu/drm/xen/xen_drm_front_kms.c:63: if (IS_ERR_OR_NULL(fb))
> drivers/gpu/drm/xen/xen_drm_front_gem.c:86: if (IS_ERR_OR_NULL(xen_obj))
> drivers/gpu/drm/xen/xen_drm_front_gem.c:120:if 
> (IS_ERR_OR_NULL(xen_obj->pages)) {
> drivers/gpu/drm/xen/xen_drm_front_gem.c:139:if (IS_ERR_OR_NULL(xen_obj))
> drivers/gpu/drm/xen/xen_drm_front_gem.c:197:if (IS_ERR_OR_NULL(xen_obj))
> drivers/gpu/drm/xen/xen_drm_front.c:403:if (IS_ERR_OR_NULL(obj)) {
>
> They're all wrong, because if the pointer was really NULL then it's
> not handled correctly and would eventually lead to a runtime failure.

It seems that you were right and I can simply change all IS_ERR_OR_NULL 
to just IS_ERR

I am planning a series of patches later on, so I'll include this fix as well

>
> Normally Smatch is smart enough to know that the pointer isn't NULL so
> it doesn't generate a warning but yesterday I introduced a bug in Smatch
> by mistake.  It's fixed now.
>
> regards,
> dan carpenter
>
Thank you,

Oleksandr

Re: [PATCH v8 04/12] x86emul: support SERIALIZE

[Jan Beulich]

On 07.05.2020 21:32, Andrew Cooper wrote:
> On 05/05/2020 09:14, Jan Beulich wrote:
>> ... enabling its use by all guest kinds at the same time.
>>
>> Signed-off-by: Jan Beulich 
> 
> Acked-by: Andrew Cooper 

Thanks.

>> @@ -5660,6 +5661,18 @@ x86_emulate(
>>  goto done;
>>  break;
>>  
>> +case 0xe8:
>> +switch ( vex.pfx )
>> +{
>> +case vex_none: /* serialize */
>> +host_and_vcpu_must_have(serialize);
>> +asm volatile ( ".byte 0x0f, 0x01, 0xe8" );
> 
> There is very little need for an actual implementation here.  The VMExit
> to get here is good enough.
> 
> The only question is whether pre-unrestricted_guest Intel boxes are
> liable to find this in real mode code.

Apart from this we also shouldn't assume HVM in the core emulator,
I think.

Jan

Re: [PATCH v8 03/12] x86emul: support ENQCMD insns

[Jan Beulich]

On 07.05.2020 20:59, Andrew Cooper wrote:
> On 05/05/2020 09:13, Jan Beulich wrote:
>> Note that the ISA extensions document revision 038 doesn't specify
>> exception behavior for ModRM.mod == 0b11; assuming #UD here.
> 
> Stale.

In which way (beyond the question of whether to use
goto unrecognized_insn in the code instead)? The doc doesn't
mention ModRM.mod specifics in any way.

>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>> @@ -11480,11 +11513,36 @@ int x86_emul_blk(
>>  {
>>  switch ( state->blk )
>>  {
>> +bool zf;
>> +
>>  /*
>>   * Throughout this switch(), memory clobbers are used to compensate
>>   * that other operands may not properly express the (full) memory
>>   * ranges covered.
>>   */
>> +case blk_enqcmd:
>> +ASSERT(bytes == 64);
>> +if ( ((unsigned long)ptr & 0x3f) )
>> +{
>> +ASSERT_UNREACHABLE();
>> +return X86EMUL_UNHANDLEABLE;
>> +}
>> +*eflags &= ~EFLAGS_MASK;
>> +#ifdef HAVE_AS_ENQCMD
>> +asm ( "enqcmds (%[src]), %[dst]" ASM_FLAG_OUT(, "; setz %0")
> 
> %[zf]

Oops, indeed.

>> +  : [zf] ASM_FLAG_OUT("=@ccz", "=qm") (zf)
>> +  : [src] "r" (data), [dst] "r" (ptr) : "memory" );
> 
> Can't src get away with being "m" (*data)?  There is no need to force it
> into a single register, even if it is overwhelmingly likely to end up
> with %rsi scheduled here.

Well, *data can't be used, as data is of void* type. It would
need to have a suitable cast on it, but since that's not
going to avoid the memory clobber I didn't think it was worth
it (also together with the comment ahead of the switch()).

>> --- a/xen/include/asm-x86/msr-index.h
>> +++ b/xen/include/asm-x86/msr-index.h
>> @@ -420,6 +420,10 @@
>>  #define MSR_IA32_TSC_DEADLINE   0x06E0
>>  #define MSR_IA32_ENERGY_PERF_BIAS   0x01b0
>>  
>> +#define MSR_IA32_PASID  0x0d93
>> +#define  PASID_PASID_MASK   0x000f
>> +#define  PASID_VALID0x8000
>> +
> 
> Above the legacy line please as this is using the newer style,

Ah, yes, I should have remembered to re-base this over your
change there.

> and drop
> _IA32.  Intel's ideal of architectural-ness isn't interesting or worth
> the added code volume.

We'd been there before, and you know I disagree. I think it
is wrong for me to make the change, but I will do so just
to retain your ack.

> PASSID_PASSID_MASK isn't great, but I can't suggest anything better, and
> MSR_PASSID_MAS doesn't work either.
> 
> Otherwise, Acked-by: Andrew Cooper 

Thanks.

Jan

Re: [PATCH v8 02/12] x86emul: support MOVDIR{I,64B} insns

[Jan Beulich]

On 07.05.2020 20:30, Andrew Cooper wrote:
> On 05/05/2020 09:13, Jan Beulich wrote:
>> Introduce a new blk() hook, paralleling the rmw() one in a certain way,
>> but being intended for larger data sizes, and hence its HVM intermediate
>> handling function doesn't fall back to splitting the operation if the
>> requested virtual address can't be mapped.
>>
>> Note that SDM revision 071 doesn't specify exception behavior for
>> ModRM.mod == 0b11; assuming #UD here.
> 
> Still stale?  It does #UD on current hardware, and will cease to #UD in
> the future when the encoding space gets used for something else.

What do you mean by "still stale"? Other insns allowing for only
memory operands have the #UD spelled out in the doc. Are you
implying by the 2nd sentence that it should rather be
"goto unrecognized_insn"? I'm afraid we're not very consistent
yet with what we do in such cases; I could certainly work
towards improving this, but the question is whether it is really
sensible in all cases to assume such partially unused encodings
may get used in the future.

>> Signed-off-by: Jan Beulich 
>> Reviewed-by: Paul Durrant 
> 
> Acked-by: Andrew Cooper 

Thanks, but as per my reply to your patch 6 comment the patch
here will need to be revised, so I'll not apply this just yet
unless you indicate up front that you're fine with me keeping it.

Jan

Re: [PATCH v8 06/12] x86/HVM: make hvmemul_blk() capable of handling r/o operations

[Jan Beulich]

On 07.05.2020 22:34, Andrew Cooper wrote:
> On 05/05/2020 09:15, Jan Beulich wrote:
>> In preparation for handling e.g. FLDENV or {F,FX,X}RSTOR here as well.
>>
>> Signed-off-by: Jan Beulich 
>> ---
>> v8: New (could be folded into "x86emul: support MOVDIR{I,64B} insns",
>> but would invalidate Paul's R-b there).
>>
>> --- a/xen/arch/x86/hvm/emulate.c
>> +++ b/xen/arch/x86/hvm/emulate.c
>> @@ -1453,7 +1453,7 @@ static int hvmemul_blk(
>>  struct hvm_emulate_ctxt *hvmemul_ctxt =
>>  container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
>>  unsigned long addr;
>> -uint32_t pfec = PFEC_page_present | PFEC_write_access;
>> +uint32_t pfec = PFEC_page_present;
>>  int rc;
>>  void *mapping = NULL;
>>  
>> @@ -1462,6 +1462,9 @@ static int hvmemul_blk(
>>  if ( rc != X86EMUL_OKAY || !bytes )
>>  return rc;
>>  
>> +if ( x86_insn_is_mem_write(state, ctxt) )
>> +pfec |= PFEC_write_access;
>> +
> 
> For the instructions with two memory operands, it conflates the
> read-only side with the read-write side.

"Conflates" is probably the wrong word? The bug you point out is
really in x86_insn_is_mem_write(), in that so far it would return
false for MOVDIR64B and ENQCMD{,S}. As a result I'll fix the
issue there while, at the same time, folding this patch into
"x86emul: support MOVDIR{I,64B} insns".

Jan

[linux-5.4 test] 150070: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-examine  4 memdisk-try-append   fail REGR. vs. 149905

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

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

version targeted for testing:
 linux592465e6a54ba8104969f3b73b58df262c5be5f5
baseline version:
 linux9895e0ac338a8060e6947f897397c21c4d78d80d

Last test of basis   149905  2020-05-02 16:38:26 Z5 days
Testing same since   150054  2020-05-06 06:41:12 Z2 days2 attempts


People who touched revisions under test:
  

[xen-4.9-testing test] 150068: regressions - trouble: fail/pass/starved

[osstest service owner]

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

Regressions :-(

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

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-qemut-ws16-amd64 16 guest-localmigrate/x10 fail pass in 
150038

Tests which did not succeed, but are not blocking:
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop   fail blocked in 149649
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop   fail blocked in 149649
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop  fail blocked in 149649
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail in 150038 blocked in 
149649
 test-amd64-amd64-xl-qemuu-ws16-amd64 16 guest-localmigrate/x10 fail in 150038 
blocked in 149649
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail in 150038 
like 149649
 test-amd64-i386-xl-qemut-ws16-amd64 16 guest-localmigrate/x10 fail in 150038 
like 149649
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 149649
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail  like 149649
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 149649
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail like 149649
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 149649
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-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-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-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  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  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-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  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-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-libvirt 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-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-arm64-arm64-xl-thunderx  2 hosts-allocate   starved  n/a

version targeted for testing:
 xen  93cc305d1f3e7c6949a8f4116446624fa2dbfdf4
baseline version:
 xen  45c90737d5f0c8bf479adcd8cb88450f1998e55c

Last test of basis   149649