[Xen-devel] [ANNOUNCE] Xen 4.13 Development Update

[Juergen Gross]

As multiple rather important patch series are very short before being ready
I have decided to push the hard code freeze one week back to October 4th.

This email only tracks big items for xen.git tree. Please reply for items you
would like to see in 4.13 so that people have an idea what is going on and
prioritise accordingly.

You're welcome to provide description and use cases of the feature you're
working on.

= Timeline =

We now adopt a fixed cut-off date scheme. We will release about every 8 months.
The upcoming 4.13 timeline are as followed:

* Last posting date: September 13th, 2019
---> We are here
* Hard code freeze: October 4th, 2019
* RC1: TBD
* Release: November 7th, 2019

Note that we don't have freeze exception scheme anymore. All patches
that wish to go into 4.13 must be posted initially no later than the
last posting date and finally no later than the hard code freeze. All
patches posted after that date will be automatically queued into next
release.

RCs will be arranged immediately after freeze.

We recently introduced a jira instance to track all the tasks (not only big)
for the project. See: https://xenproject.atlassian.net/projects/XEN/issues.

Some of the tasks tracked by this e-mail also have a corresponding jira task
referred by XEN-N.

I have started to include the version number of series associated to each
feature. Can each owner send an update on the version number if the series
was posted upstream?

= Projects =

== Hypervisor == 

*  Core scheduling (v4)
  -  Juergen Gross

=== x86 === 

*  Intel Processor Trace virtualization enabling (v1)
  -  Luwei Kang

*  Linux stub domains (RFC v2)
  -  Marek Marczykowski-Górecki

*  Improve late microcode loading (v12)
  -  Chao Gao

*  Fixes to #DB injection
  -  Andrew Cooper

*  CPUID/MSR Xen/toolstack improvements
  -  Andrew Cooper

*  Improvements to domain_crash()
  -  Andrew Cooper

*  EIBRS
  -  Andrew Cooper

*  Xen ioreq server (v2)
  -  Roger Pau Monne

=== ARM === 

== Completed == 

*  Drop tmem
  -  Wei Liu

*  Add support for Hygon Dhyana Family 18h processor
  -  Pu Wen

*  hypervisor x86 instruction emulator additions for AVX512
  -  Jan Beulich

*  x2APIC support for AMD
  -  Jan Beulich

*  add per-domain IOMMU control
  -  Paul Durrant

*  TEE mediator (and OP-TEE) support in XEN
  -  Volodymyr Babchuk

*  Renesas IPMMU-VMSA support + Linux's iommu_fwspec
  -  Oleksandr Tyshchenko


Juergen Gross

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

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

[Roman Shaposhnik]

On Thu, Sep 26, 2019 at 12:44 AM Jan Beulich  wrote:
>
> On 26.09.2019 00:31, Roman Shaposhnik wrote:
> > Jan, Roger, thank you so much for the initial ideas. I tried a few of
> > those and here's where I am.
> >
> > First of all, it is definitely related to CPU bring up. Adding
> > cpuidle=0 to xen command line made Xen boot.
> >
> > Then, a good friend of mine (who you may know from ancient Xen days
> > ;-)) suggested that this could be related to this:
> >  https://wiki.xenproject.org/wiki/Xen_power_management
> > so I went to the BIOS settings and quite to my surprise all of them
> > were grayed out (not tweakable).
> >
> > The only one that wasn't was 2xAPIC support. So just for kicks -- I
> > disabled that.
> >
> > That, in turn, made Xen boot even without cpuidle=0. I'm attaching that log.
>
> Interesting, but unfortunately this particular log is of no real use
> for investigation of the issue (other than knowing the CPU model). I
> also notice it's a 4.12.0 log, when your original report was against
> latest master.
>
> > So I guess at this point, you could say that I have a functional
> > system, but I'm curious whether you guys would be interested to look
> > into 2xAPIC situation.
>
> Of course we do. As a next step I'd suggest reverting the BIOS settings
> change you did, and instead using the "x2apic=0" Xen command line option.

Interestingly enough, this doesn't really solve the problem completely.
Specifying x2apic=0 certainly makes Xen go much further to a point
where it tries to load Dom0 and then the console VGA screen goes
blank (this is where that serial debug output would be very useful :-().

I suppose I can buy:
 
https://www.cdw.com/product/hpe-dl20-ml30-gen10-m.2-dedicated-ilo-and-serial-port-kit/5348024
but that'll take some time to arrive.

> And then we of course need a complete boot log (as requested earlier) of
> a problem case.
>
> Further I'd suggest moving away from the black-and-white "cpuidle="
> option, and instead limiting use of deep C states ("max_cstate="). I
> wouldn't be surprised if this was the issue; we'd then have to first
> of all go through errata for the part your system is using.

Yup. max_cstate=1 makes it boot fine. max_cstate=2 though hangs
the system *exactly* in the same way as specifying x2apic=0
(which is different from the original problem as I've described above).

Can you please elaborate on "we'd then have to first of all go through
errata for the part your system is using"

Thanks,
Roman.

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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-freebsd10-i386 11 guest-startfail REGR. vs. 140282
 test-amd64-i386-freebsd10-amd64 11 guest-start   fail REGR. vs. 140282
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail 
REGR. vs. 140282
 test-amd64-i386-qemuu-rhel6hvm-amd 10 redhat-install fail REGR. vs. 140282
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 
140282
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail REGR. vs. 140282
 test-amd64-i386-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 
140282
 test-amd64-amd64-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 
140282
 test-amd64-i386-qemuu-rhel6hvm-intel 10 redhat-install   fail REGR. vs. 140282
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail 
REGR. vs. 140282
 test-amd64-amd64-qemuu-nested-amd 10 debian-hvm-install  fail REGR. vs. 140282
 test-amd64-amd64-xl-qemuu-win7-amd64 10 windows-install  fail REGR. vs. 140282
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 10 debian-hvm-install fail REGR. 
vs. 140282
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 10 debian-hvm-install fail REGR. 
vs. 140282
 test-amd64-amd64-qemuu-nested-intel 10 debian-hvm-install fail REGR. vs. 140282
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail 
REGR. vs. 140282
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail REGR. vs. 140282
 test-amd64-i386-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 140282
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail 
REGR. vs. 140282
 test-amd64-i386-xl-qemuu-win7-amd64 10 windows-install   fail REGR. vs. 140282
 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-install  fail REGR. vs. 140282
 test-amd64-i386-xl-qemuu-ws16-amd64 10 windows-install   fail REGR. vs. 140282
 test-armhf-armhf-xl-arndale   7 xen-boot fail REGR. vs. 140282

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 140282
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 140282
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  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-arm64-arm64-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-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-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-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 

[Xen-devel] [libvirt test] 141893: tolerable FAIL - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-libvirt-qcow2 15 guest-start/debian.repeat   fail like 141859
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 141859
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 141859
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-qcow2 12 migrate-support-checkfail never pass
 test-arm64-arm64-libvirt-qcow2 13 saverestore-support-checkfail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass

version targeted for testing:
 libvirt  742f59903349cefdd02f58e25eed3747c5165e63
baseline version:
 libvirt  03449e25047c71eaf4a54b6459007bf2c504802e

Last test of basis   141859  2019-09-26 08:22:01 Z1 days
Testing same since   141893  2019-09-27 09:03:15 Z0 days1 attempts


People who touched revisions under test:
  Daniel Henrique Barboza 
  Laine Stump 
  Michal Privoznik 
  Peter Krempa 

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



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

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

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
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/libvirt.git
   03449e2504..742f599033  742f59903349cefdd02f58e25eed3747c5165e63 -> 
xen-tested-master

___
Xen-devel mailing 

Re: [Xen-devel] [PATCH V6 0/8] iommu/arm: Add Renesas IPMMU-VMSA support + Linux's iommu_fwspec

[Oleksandr Tyshchenko]

Hi Stefano

Sorry for the possible format issues.


сб, 28 сент. 2019 г., 01:50 Stefano Stabellini :

> On Thu, 26 Sep 2019, Oleksandr wrote:
> > On 26.09.19 17:56, Julien Grall wrote:
> > > Hi,
> >
> > Hi Julien
> >
> >
> > >
> > > On 9/26/19 12:20 PM, Oleksandr Tyshchenko wrote:
> > > > Oleksandr Tyshchenko (8):
> > > >iommu/arm: Add iommu_helpers.c file to keep common for IOMMUs
> stuff
> > > >iommu/arm: Add ability to handle deferred probing request
> > > >xen/common: Introduce _xrealloc function
> > > >xen/common: Introduce xrealloc_flex_struct() helper macros
> > > >iommu/arm: Add lightweight iommu_fwspec support
> > > >iommu: Order the headers alphabetically in device_tree.c
> > > >iommu/arm: Introduce iommu_add_dt_device API
> > > >iommu/arm: Add Renesas IPMMU-VMSA support
> > >
> > > This series is now merged.
> >
> > Thank you!
>
> I just wanted to provide early feedback that this series causes problems
> with the legacy mmu-masters binding:
>

This series was developed in a way to add new functionality, but not to
brake existing (legacy bindings). Probably, I missed something
important. iommu_add_dt_device() could return an error (I assume, this is
what you are facing) if the device node in DT contains "iommus" property (I
mean, uses new bindings), but the IOMMU driver doesn't implement required
callbacks yet. Do the device nodes in your DT contain "iommus" property?
And to which domain these devices (in your log) are going to be assigned
(dom0 or other domains?).


> (XEN) Failed to add /amba/dma@fd50 to the IOMMU
> (XEN) Failed to add /amba/dma@fd51 to the IOMMU
> (XEN) Failed to add /amba/dma@fd52 to the IOMMU
> (XEN) Failed to add /amba/dma@fd53 to the IOMMU
> (XEN) Failed to add /amba/dma@fd54 to the IOMMU
> (XEN) Failed to add /amba/dma@fd55 to the IOMMU
> (XEN) Failed to add /amba/dma@fd56 to the IOMMU
> (XEN) Failed to add /amba/dma@fd57 to the IOMMU
> (XEN) Failed to add /amba/dma@ffa8 to the IOMMU
> (XEN) Failed to add /amba/dma@ffa9 to the IOMMU
> (XEN) Failed to add /amba/dma@ffaa to the IOMMU
> (XEN) Failed to add /amba/dma@ffab to the IOMMU
> (XEN) Failed to add /amba/dma@ffac to the IOMMU
> (XEN) Failed to add /amba/dma@ffad to the IOMMU
> (XEN) Failed to add /amba/dma@ffae to the IOMMU
> (XEN) Failed to add /amba/dma@ffaf to the IOMMU
> (XEN) Failed to add /amba/nand@ff10 to the IOMMU
> (XEN) Failed to add /amba/ethernet@ff0b to the IOMMU
> (XEN) Failed to add /amba/ethernet@ff0c to the IOMMU
> (XEN) Failed to add /amba/ethernet@ff0d to the IOMMU
> (XEN) Failed to add /amba/spi@ff0f to the IOMMU
> (XEN) Failed to add /amba/sdhci@ff16 to the IOMMU
> (XEN) Failed to add /amba/sdhci@ff17 to the IOMMU
> (XEN) Failed to add /amba/usb@fe20 to the IOMMU
> (XEN) Failed to add /amba/usb@fe30 to the IOMMU
>
> Booting fails and also even forcing to continue devices are unusable. I
> haven't managed to investigate further.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v5 5/8] xen/arm: assign devices to boot domains

[Stefano Stabellini]

On Fri, 27 Sep 2019, Julien Grall wrote:
> Hi,
> 
> On 27/09/2019 15:40, Oleksandr wrote:
> > 
> > On 26.09.19 00:12, Julien Grall wrote:
> >> On 25/09/2019 19:49, Stefano Stabellini wrote:
> >>> Scan the user provided dtb fragment at boot. For each device node, map
> >>> memory to guests, and route interrupts and setup the iommu.
> >>>
> >>> The memory region to remap is specified by the "xen,reg" property.
> >>>
> >>> The iommu is setup by passing the node of the device to assign on the
> >>> host device tree. The path is specified in the device tree fragment as
> >>> the "xen,path" string property.
> >>>
> >>> The interrupts are remapped based on the information from the
> >>> corresponding node on the host device tree. Call
> >>> handle_device_interrupts to remap interrupts. Interrupts related device
> >>> tree properties are copied from the device tree fragment, same as all
> >>> the other properties.
> >>>
> >>> Also add the new flag XEN_DOMCTL_CDF_iommu to that dom0less domU can use
> >>> the IOMMU.
> >>>
> >>> Signed-off-by: Stefano Stabellini 
> >>> ---
> >>> Changes in v5:
> >>> - use local variable for name
> >>> - use map_regions_p2mt
> >>> - add warning for not page aligned addresses/sizes
> >>> - introduce handle_passthrough_prop
> >>>
> >>> Changes in v4:
> >>> - use unsigned
> >>> - improve commit message
> >>> - code style
> >>> - use dt_prop_cmp
> >>> - use device_tree_get_reg
> >>> - don't copy over xen,reg and xen,path
> >>> - don't create special interrupt properties for domU: copy them from the
> >>>     fragment
> >>> - in-code comment
> >>>
> >>> Changes in v3:
> >>> - improve commit message
> >>> - remove superfluous cast
> >>> - merge code with the copy code
> >>> - add interrup-parent
> >>> - demove depth > 2 check
> >>> - reuse code from handle_device_interrupts
> >>> - copy interrupts from host dt
> >>>
> >>> Changes in v2:
> >>> - rename "path" to "xen,path"
> >>> - grammar fix
> >>> - use gaddr_to_gfn and maddr_to_mfn
> >>> - remove depth <= 2 limitation in scanning the dtb fragment
> >>> - introduce and parse xen,reg
> >>> - code style
> >>> - support more than one interrupt per device
> >>> - specify only the GIC is supported
> >>> ---
> >>>    xen/arch/arm/domain_build.c | 101 
> >>> ++--
> >>>    1 file changed, 97 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> >>> index 9d985d3bbe..414893bc24 100644
> >>> --- a/xen/arch/arm/domain_build.c
> >>> +++ b/xen/arch/arm/domain_build.c
> >>> @@ -1701,6 +1701,85 @@ static int __init make_vpl011_uart_node(struct 
> >>> kernel_info *kinfo)
> >>>    }
> >>>    #endif
> >>> +/*
> >>> + * Scan device tree properties for passthrough specific information.
> >>> + * Returns -ENOENT when no passthrough properties are found
> >>> + * < 0 on error
> >>> + * 0 on success
> >>> + */
> >>> +static int __init handle_passthrough_prop(struct kernel_info *kinfo,
> >>> +  const struct fdt_property 
> >>> *prop,
> >>> +  const char *name,
> >>> +  uint32_t address_cells, 
> >>> uint32_t size_cells)
> >>> +{
> >>> +    const __be32 *cell;
> >>> +    unsigned int i, len;
> >>> +    struct dt_device_node *node;
> >>> +    int res;
> >>> +
> >>> +    /* xen,reg specifies where to map the MMIO region */
> >>> +    if ( dt_prop_cmp("xen,reg", name) == 0 )
> >>> +    {
> >>> +    paddr_t mstart, size, gstart;
> >>> +    cell = (const __be32 *)prop->data;
> >>> +    len = fdt32_to_cpu(prop->len) /
> >>> +    ((address_cells * 2 + size_cells) * sizeof(uint32_t));
> >>> +
> >>> +    for ( i = 0; i < len; i++ )
> >>> +    {
> >>> +    device_tree_get_reg(, address_cells, size_cells,
> >>> +    , );
> >>> +    gstart = dt_next_cell(address_cells, );
> >>> +
> >>> +    if ( gstart & ~PAGE_MASK || mstart & ~PAGE_MASK || size 
> >>> & ~PAGE_MASK )
> >>> +    dprintk(XENLOG_WARNING,
> >>> +    "DomU passthrough config has not page 
> >>> aligned addresses/sizes\n");
> >> I don't think this is wise to continue, the more this is a printk that
> >> can only happen in debug build. So someone using a release build may not
> >> notice the error.
> >>
> >> So I think this wants to be a printk(XENLOG_ERR,...) and also return an
> >> error.
> >>
> >>> +
> >>> +    res = map_regions_p2mt(kinfo->d,
> >>> +    gaddr_to_gfn(gstart),
> >>> +    PFN_DOWN(size),
> >>> +    maddr_to_mfn(mstart),
> >>> +    p2m_mmio_direct_dev);
> >> Coding style.
> >>
> >>> +    if ( res < 0 )
> >>> +    {
> >>> +    dprintk(XENLOG_ERR,
> >>> +    "Failed to map %"PRIpaddr" to the guest 
> >>> at%"PRIpaddr"\n",
> >>> +    mstart, 

[Xen-devel] [freebsd-master test] 141895: regressions - trouble: blocked/fail

[osstest service owner]

flight 141895 freebsd-master real [real]
http://logs.test-lab.xenproject.org/osstest/logs/141895/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-freebsd   7 freebsd-buildfail REGR. vs. 141501

Tests which did not succeed, but are not blocking:
 build-amd64-xen-freebsd   1 build-check(1)   blocked  n/a
 build-amd64-freebsd-again 1 build-check(1)   blocked  n/a

version targeted for testing:
 freebsd  0eeddc2571a9fe19bd0efdb9ec602c9487ceb7b5
baseline version:
 freebsd  14aef6dfca96006e52b8fb920bde7c612ba58b79

Last test of basis   141501  2019-09-20 09:19:51 Z7 days
Failing since141701  2019-09-23 09:19:41 Z4 days3 attempts
Testing same since   141895  2019-09-27 09:20:38 Z0 days1 attempts


People who touched revisions under test:
  0mp <0...@freebsd.org>
  alc 
  allanjude 
  avg 
  bapt 
  cem 
  cy 
  dab 
  daichi 
  dim 
  emaste 
  erj 
  gallatin 
  gjb 
  glebius 
  gonzo 
  grembo 
  hrs 
  hselasky 
  imp 
  jhibbits 
  jilles 
  jkim 
  jtl 
  karels 
  kevans 
  kib 
  lwhsu 
  markj 
  mav 
  mhorne 
  mjg 
  mm 
  olivier 
  Piotr Pietruszewski 
  rmacklem 
  rrs 
  sef 
  tijl 
  tsoome 
  tuexen 
  yuripv 

jobs:
 build-amd64-freebsd-againblocked 
 build-amd64-freebsd  fail
 build-amd64-xen-freebsd  blocked 



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

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

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

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


Not pushing.

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

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

Re: [Xen-devel] [PATCH V6 0/8] iommu/arm: Add Renesas IPMMU-VMSA support + Linux's iommu_fwspec

[Stefano Stabellini]

On Thu, 26 Sep 2019, Oleksandr wrote:
> On 26.09.19 17:56, Julien Grall wrote:
> > Hi,
> 
> Hi Julien
> 
> 
> > 
> > On 9/26/19 12:20 PM, Oleksandr Tyshchenko wrote:
> > > Oleksandr Tyshchenko (8):
> > >    iommu/arm: Add iommu_helpers.c file to keep common for IOMMUs stuff
> > >    iommu/arm: Add ability to handle deferred probing request
> > >    xen/common: Introduce _xrealloc function
> > >    xen/common: Introduce xrealloc_flex_struct() helper macros
> > >    iommu/arm: Add lightweight iommu_fwspec support
> > >    iommu: Order the headers alphabetically in device_tree.c
> > >    iommu/arm: Introduce iommu_add_dt_device API
> > >    iommu/arm: Add Renesas IPMMU-VMSA support
> > 
> > This series is now merged.
> 
> Thank you!

I just wanted to provide early feedback that this series causes problems
with the legacy mmu-masters binding:

(XEN) Failed to add /amba/dma@fd50 to the IOMMU
(XEN) Failed to add /amba/dma@fd51 to the IOMMU
(XEN) Failed to add /amba/dma@fd52 to the IOMMU
(XEN) Failed to add /amba/dma@fd53 to the IOMMU
(XEN) Failed to add /amba/dma@fd54 to the IOMMU
(XEN) Failed to add /amba/dma@fd55 to the IOMMU
(XEN) Failed to add /amba/dma@fd56 to the IOMMU
(XEN) Failed to add /amba/dma@fd57 to the IOMMU
(XEN) Failed to add /amba/dma@ffa8 to the IOMMU
(XEN) Failed to add /amba/dma@ffa9 to the IOMMU
(XEN) Failed to add /amba/dma@ffaa to the IOMMU
(XEN) Failed to add /amba/dma@ffab to the IOMMU
(XEN) Failed to add /amba/dma@ffac to the IOMMU
(XEN) Failed to add /amba/dma@ffad to the IOMMU
(XEN) Failed to add /amba/dma@ffae to the IOMMU
(XEN) Failed to add /amba/dma@ffaf to the IOMMU
(XEN) Failed to add /amba/nand@ff10 to the IOMMU
(XEN) Failed to add /amba/ethernet@ff0b to the IOMMU
(XEN) Failed to add /amba/ethernet@ff0c to the IOMMU
(XEN) Failed to add /amba/ethernet@ff0d to the IOMMU
(XEN) Failed to add /amba/spi@ff0f to the IOMMU
(XEN) Failed to add /amba/sdhci@ff16 to the IOMMU
(XEN) Failed to add /amba/sdhci@ff17 to the IOMMU
(XEN) Failed to add /amba/usb@fe20 to the IOMMU
(XEN) Failed to add /amba/usb@fe30 to the IOMMU

Booting fails and also even forcing to continue devices are unusable. I
haven't managed to investigate further.___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [xen-unstable test] 141882: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-libvirt 12 guest-start  fail REGR. vs. 141822
 test-amd64-amd64-libvirt-pair 22 guest-migrate/src_host/dst_host fail REGR. 
vs. 141822
 test-amd64-i386-libvirt-pair 22 guest-migrate/src_host/dst_host fail REGR. vs. 
141822
 test-arm64-arm64-examine11 examine-serial/bootloader fail REGR. vs. 141822
 test-arm64-arm64-libvirt-xsm 12 guest-start  fail REGR. vs. 141822
 test-amd64-amd64-libvirt-vhd 10 debian-di-installfail REGR. vs. 141822
 test-armhf-armhf-libvirt 12 guest-start  fail REGR. vs. 141822
 test-armhf-armhf-libvirt-raw 10 debian-di-installfail REGR. vs. 141822

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

version targeted for testing:
 xen  ddc5a85fbcfbacc34bbd9abcdb12923de2fc27b3
baseline version:
 xen  f93abf0315efef861270c25d83c8047fd6a54ec4

Last test of basis   141822  2019-09-25 14:59:55 Z2 days
Testing same since   141882  2019-09-27 01:20:55 Z0 days1 

Re: [Xen-devel] [PATCH RESEND v1 1/8] stubdom/vtpm: include stdio.h for declaration of printf

[Daniel De Graaf]

On 9/24/19 10:03 AM, Olaf Hering wrote:

The function read_vtpmblk uses printf(3), but stdio.h is not included
in this file. This results in a warning from gcc-7:

vtpmblk.c: In function 'read_vtpmblk':
vtpmblk.c:322:7: warning: implicit declaration of function 'printf' 
[-Wimplicit-function-declaration]
printf("Expected: ");
vtpmblk.c:322:7: warning: incompatible implicit declaration of built-in 
function 'printf'
vtpmblk.c:322:7: note: include '' or provide a declaration of 'printf'

Signed-off-by: Olaf Hering 

Acked-by: Daniel De Graaf 

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

Re: [Xen-devel] [PATCH v5 5/8] xen/arm: assign devices to boot domains (fwd)

[Stefano Stabellini]

Hi Juergen,

This email is about the dom0less device assignment patch series. It is
very close to ready. I sent v6 yesterday addressing the remaining
comments from Julien. We are down to smaller details.

Oleksandr's iommu series got merged today. That requires a small changes
to the patch below.

I plan to send a v7 addressing the "merge conflict" and also addressing
any remaining points from Julien (Julien plans to give it a look before
Monday my time.)

I hope that's OK for you if we merge this series early next week.

Cheers,

Stefano

-- Forwarded message --
Date: Fri, 27 Sep 2019 20:02:56 +
From: Julien Grall 
To: Oleksandr , Stefano Stabellini 
Cc: nd , Stefano Stabellini ,
"andrii_ani...@epam.com" ,
Achin Gupta ,
"xen-de...@lists.xen.org" ,
"volodymyr_babc...@epam.com" 
Subject: Re: [Xen-devel] [PATCH v5 5/8] xen/arm: assign devices to boot domains

Hi,

On 27/09/2019 15:40, Oleksandr wrote:
> 
> On 26.09.19 00:12, Julien Grall wrote:
>> On 25/09/2019 19:49, Stefano Stabellini wrote:
>>> Scan the user provided dtb fragment at boot. For each device node, map
>>> memory to guests, and route interrupts and setup the iommu.
>>>
>>> The memory region to remap is specified by the "xen,reg" property.
>>>
>>> The iommu is setup by passing the node of the device to assign on the
>>> host device tree. The path is specified in the device tree fragment as
>>> the "xen,path" string property.
>>>
>>> The interrupts are remapped based on the information from the
>>> corresponding node on the host device tree. Call
>>> handle_device_interrupts to remap interrupts. Interrupts related device
>>> tree properties are copied from the device tree fragment, same as all
>>> the other properties.
>>>
>>> Also add the new flag XEN_DOMCTL_CDF_iommu to that dom0less domU can use
>>> the IOMMU.
>>>
>>> Signed-off-by: Stefano Stabellini 
>>> ---
>>> Changes in v5:
>>> - use local variable for name
>>> - use map_regions_p2mt
>>> - add warning for not page aligned addresses/sizes
>>> - introduce handle_passthrough_prop
>>>
>>> Changes in v4:
>>> - use unsigned
>>> - improve commit message
>>> - code style
>>> - use dt_prop_cmp
>>> - use device_tree_get_reg
>>> - don't copy over xen,reg and xen,path
>>> - don't create special interrupt properties for domU: copy them from the
>>>     fragment
>>> - in-code comment
>>>
>>> Changes in v3:
>>> - improve commit message
>>> - remove superfluous cast
>>> - merge code with the copy code
>>> - add interrup-parent
>>> - demove depth > 2 check
>>> - reuse code from handle_device_interrupts
>>> - copy interrupts from host dt
>>>
>>> Changes in v2:
>>> - rename "path" to "xen,path"
>>> - grammar fix
>>> - use gaddr_to_gfn and maddr_to_mfn
>>> - remove depth <= 2 limitation in scanning the dtb fragment
>>> - introduce and parse xen,reg
>>> - code style
>>> - support more than one interrupt per device
>>> - specify only the GIC is supported
>>> ---
>>>    xen/arch/arm/domain_build.c | 101 
>>> ++--
>>>    1 file changed, 97 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>>> index 9d985d3bbe..414893bc24 100644
>>> --- a/xen/arch/arm/domain_build.c
>>> +++ b/xen/arch/arm/domain_build.c
>>> @@ -1701,6 +1701,85 @@ static int __init make_vpl011_uart_node(struct 
>>> kernel_info *kinfo)
>>>    }
>>>    #endif
>>> +/*
>>> + * Scan device tree properties for passthrough specific information.
>>> + * Returns -ENOENT when no passthrough properties are found
>>> + * < 0 on error
>>> + * 0 on success
>>> + */
>>> +static int __init handle_passthrough_prop(struct kernel_info *kinfo,
>>> +  const struct fdt_property 
>>> *prop,
>>> +  const char *name,
>>> +  uint32_t address_cells, 
>>> uint32_t size_cells)
>>> +{
>>> +    const __be32 *cell;
>>> +    unsigned int i, len;
>>> +    struct dt_device_node *node;
>>> +    int res;
>>> +
>>> +    /* xen,reg specifies where to map the MMIO region */
>>> +    if ( dt_prop_cmp("xen,reg", name) == 0 )
>>> +    {
>>> +    paddr_t mstart, size, gstart;
>>> +    cell = (const __be32 *)prop->data;
>>> +    len = fdt32_to_cpu(prop->len) /
>>> +    ((address_cells * 2 + size_cells) * sizeof(uint32_t));
>>> +
>>> +    for ( i = 0; i < len; i++ )
>>> +    {
>>> +    device_tree_get_reg(, address_cells, size_cells,
>>> +    , );
>>> +    gstart = dt_next_cell(address_cells, );
>>> +
>>> +    if ( gstart & ~PAGE_MASK || mstart & ~PAGE_MASK || size 
>>> & ~PAGE_MASK )
>>> +    dprintk(XENLOG_WARNING,
>>> +    "DomU passthrough config has not page 
>>> aligned addresses/sizes\n");
>> I don't think this is wise to continue, the more this is a printk that
>> can only happen in debug build. So someone using a release 

Re: [Xen-devel] [PATCH RFC for-4.13 04/10] xen/arm: Ensure the SSBD workaround is re-enabled right after exiting a guest

[Julien Grall]

Hi,

On 27/09/2019 18:58, Volodymyr Babchuk wrote:
> 
> Julien Grall writes:
> 
>> Hi,
>>
>> On 27/09/2019 15:21, Volodymyr Babchuk wrote:
>>>
>>> Julien Grall writes:
>>>
 On 27/09/2019 14:33, Volodymyr Babchuk wrote:
> Julien Grall writes:
>> On 27/09/2019 13:39, Volodymyr Babchuk wrote:
>>> Julien Grall writes:
 On 27/09/2019 12:56, Volodymyr Babchuk wrote:
> Julien Grall writes:
>>> Or maybe, we should not split the function at all? Instead, we enable
>>> interrupts right in the middle of it.
>>
>> I thought about this but I didn't much like the resulting code.
>>
>> The instruction to unmask interrupts requires to take an immediate
>> (indicates which interrupts to unmask). As not all the traps require
>> to unmask the same interrupts, we would end up to have to a bunch of
>> if in the code to select the right unmasking.
> Ah, yes, this is the problem. We can provide callback to
> enter_hypervisor_from_guest().

 I am not sure what you mean by this. Do you mean a callback that will
 unmask the interrupts?
>>> Yes. You can pass function pointer to enter_hypervisor_from_guest(). To
>>> a function, that will unmask the interrupts. I'm sure that guest_vector
>>> macro can generate it for you. Something like this:
>>>
>>>   .macro  guest_vector compat, iflags, trap, save_x0_x1=1
>>>   entry   hyp=0, compat=\compat, save_x0_x1=\save_x0_x1
>>>   /*
>>>* The vSError will be checked while 
>>> SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
>>>* is not set. If a vSError took place, the initial exception 
>>> will be
>>>* skipped. Exit ASAP
>>>*/
>>>   ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
>>>   "nop; nop",
>>>   SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
>>>   ldr x0, =1f
>>>   bl  enter_hypervisor_from_guest
>>>   mov x0, sp
>>>   bl  do_trap_\trap
>>>   b   1f
>>> 2:
>>>   msr daifclr, \iflags
>>>   ret
>>> 1:
>>>   exithyp=0, compat=\compat
>>>   .endm
>>
>> TBH, I don't see what's the point you are trying to make here. Yes,
>> there are many way to write a code and possibility to make one
>> function. You could also create a skeleton macro for
>> enter_hypervisor_from_guest and generate N of them (one per set of
>> unmask interrupts) and call them.
>>
>> But are they really worth it?
> The point is that you are trying to split one entity into two.
> As I see it: semantically we have one function:
> enter_hypervisor_from_guest(). The purpose of this function is clear:
> finish transition from guest mode to hypervisor mode.
> 
> But because of some architectural limitation (daifclr requires only
> immediate argument) you are forced to divide this function in two.
> I don't like this, because entry path now is more complex. To follow
> what is going you need to bounce between head.S and traps.c one extra time.

Ok. If I understand correctly, this is mostly a matter of taste. Right?

I am going to ignore the "matter of taste" and just focus on the code 
itself. While I quite like the idea of a single function, I have two 
concerns with this:
1) Because this is a callback, you will use an indirect branch. The 
address used is loaded from the literal pool (ldr x0, =...), therefore 
your branch will depend on a load. Such construction may stall the 
pipeline for a long time as most likely you will have to fetch the 
address from memory and not the cache (the cache is likely to be 
populated with mostly guest stuff). Depending on the core, this may have 
quite an impact. I am aware that we have been using in quite a few 
places such pattern within Xen but we are trying to get away. For 
instance, on x86 they recently introduced a way to converting indirect 
branch to direct branch if the address is fixed after boot (see the 
alternative_call macro).

2) With the split functions, it is easier to spot in a diff if 
someone is trying to add code before the interrupts are unmasked. So I 
feel this is more maintainable as I have one less thing to worry when 
reviewing.

The second one is borderline matter of taste, so it is less important. 
But the first one is important to me.

So any solution should address this.

Cheers,

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

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

[osstest service owner]

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

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  7a4e674905b3cbbe48e81c3222361a7f3579
baseline version:
 xen  fab256ba5feee451a07c6cb4fe3ec12d2d7a5b41

Last test of basis   141902  2019-09-27 13:00:45 Z0 days
Testing same since   141909  2019-09-27 17:03:48 Z0 days1 attempts


People who touched revisions under test:
  Jan Beulich 
  Juergen Gross 
  Julien Grall 
  Marek Marczykowski-Górecki 
  Simon Gaiser 

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
   fab256ba5f..7a4e67  7a4e674905b3cbbe48e81c3222361a7f3579 -> smoke

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

Re: [Xen-devel] [PATCH v5 5/8] xen/arm: assign devices to boot domains

[Julien Grall]

Hi,

On 27/09/2019 15:40, Oleksandr wrote:
> 
> On 26.09.19 00:12, Julien Grall wrote:
>> On 25/09/2019 19:49, Stefano Stabellini wrote:
>>> Scan the user provided dtb fragment at boot. For each device node, map
>>> memory to guests, and route interrupts and setup the iommu.
>>>
>>> The memory region to remap is specified by the "xen,reg" property.
>>>
>>> The iommu is setup by passing the node of the device to assign on the
>>> host device tree. The path is specified in the device tree fragment as
>>> the "xen,path" string property.
>>>
>>> The interrupts are remapped based on the information from the
>>> corresponding node on the host device tree. Call
>>> handle_device_interrupts to remap interrupts. Interrupts related device
>>> tree properties are copied from the device tree fragment, same as all
>>> the other properties.
>>>
>>> Also add the new flag XEN_DOMCTL_CDF_iommu to that dom0less domU can use
>>> the IOMMU.
>>>
>>> Signed-off-by: Stefano Stabellini 
>>> ---
>>> Changes in v5:
>>> - use local variable for name
>>> - use map_regions_p2mt
>>> - add warning for not page aligned addresses/sizes
>>> - introduce handle_passthrough_prop
>>>
>>> Changes in v4:
>>> - use unsigned
>>> - improve commit message
>>> - code style
>>> - use dt_prop_cmp
>>> - use device_tree_get_reg
>>> - don't copy over xen,reg and xen,path
>>> - don't create special interrupt properties for domU: copy them from the
>>>     fragment
>>> - in-code comment
>>>
>>> Changes in v3:
>>> - improve commit message
>>> - remove superfluous cast
>>> - merge code with the copy code
>>> - add interrup-parent
>>> - demove depth > 2 check
>>> - reuse code from handle_device_interrupts
>>> - copy interrupts from host dt
>>>
>>> Changes in v2:
>>> - rename "path" to "xen,path"
>>> - grammar fix
>>> - use gaddr_to_gfn and maddr_to_mfn
>>> - remove depth <= 2 limitation in scanning the dtb fragment
>>> - introduce and parse xen,reg
>>> - code style
>>> - support more than one interrupt per device
>>> - specify only the GIC is supported
>>> ---
>>>    xen/arch/arm/domain_build.c | 101 
>>> ++--
>>>    1 file changed, 97 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>>> index 9d985d3bbe..414893bc24 100644
>>> --- a/xen/arch/arm/domain_build.c
>>> +++ b/xen/arch/arm/domain_build.c
>>> @@ -1701,6 +1701,85 @@ static int __init make_vpl011_uart_node(struct 
>>> kernel_info *kinfo)
>>>    }
>>>    #endif
>>> +/*
>>> + * Scan device tree properties for passthrough specific information.
>>> + * Returns -ENOENT when no passthrough properties are found
>>> + * < 0 on error
>>> + * 0 on success
>>> + */
>>> +static int __init handle_passthrough_prop(struct kernel_info *kinfo,
>>> +  const struct fdt_property 
>>> *prop,
>>> +  const char *name,
>>> +  uint32_t address_cells, 
>>> uint32_t size_cells)
>>> +{
>>> +    const __be32 *cell;
>>> +    unsigned int i, len;
>>> +    struct dt_device_node *node;
>>> +    int res;
>>> +
>>> +    /* xen,reg specifies where to map the MMIO region */
>>> +    if ( dt_prop_cmp("xen,reg", name) == 0 )
>>> +    {
>>> +    paddr_t mstart, size, gstart;
>>> +    cell = (const __be32 *)prop->data;
>>> +    len = fdt32_to_cpu(prop->len) /
>>> +    ((address_cells * 2 + size_cells) * sizeof(uint32_t));
>>> +
>>> +    for ( i = 0; i < len; i++ )
>>> +    {
>>> +    device_tree_get_reg(, address_cells, size_cells,
>>> +    , );
>>> +    gstart = dt_next_cell(address_cells, );
>>> +
>>> +    if ( gstart & ~PAGE_MASK || mstart & ~PAGE_MASK || size 
>>> & ~PAGE_MASK )
>>> +    dprintk(XENLOG_WARNING,
>>> +    "DomU passthrough config has not page 
>>> aligned addresses/sizes\n");
>> I don't think this is wise to continue, the more this is a printk that
>> can only happen in debug build. So someone using a release build may not
>> notice the error.
>>
>> So I think this wants to be a printk(XENLOG_ERR,...) and also return an
>> error.
>>
>>> +
>>> +    res = map_regions_p2mt(kinfo->d,
>>> +    gaddr_to_gfn(gstart),
>>> +    PFN_DOWN(size),
>>> +    maddr_to_mfn(mstart),
>>> +    p2m_mmio_direct_dev);
>> Coding style.
>>
>>> +    if ( res < 0 )
>>> +    {
>>> +    dprintk(XENLOG_ERR,
>>> +    "Failed to map %"PRIpaddr" to the guest 
>>> at%"PRIpaddr"\n",
>>> +    mstart, gstart);
>> Similarly, this wants to be a printk.
>>
>>> +    return -EFAULT;
>>> +    }
>>> +    }
>>> +
>>> +    return 0;
>>> +    }
>>> +    /*
>>> + * xen,path specifies the corresponding node in the host DT.
>>> + * Both interrupt mappings and IOMMU settings are 

Re: [Xen-devel] [PATCH v1] xen/balloon: Set pages PageOffline() in balloon_add_region()

[Boris Ostrovsky]

On 9/27/19 11:46 AM, David Hildenbrand wrote:
> We are missing a __SetPageOffline(), which is why we can get
> !PageOffline() pages onto the balloon list, where
> alloc_xenballooned_pages() will complain:
>
> page:ea0003e7ffc0 refcount:1 mapcount:0 mapping: index:0x0
> flags: 0xe1000(reserved)
> raw: 000e1000 dead0100 dead0200 
> raw:   0001 
> page dumped because: VM_BUG_ON_PAGE(!PageOffline(page))
> [ cut here ]
> kernel BUG at include/linux/page-flags.h:744!
> invalid opcode:  [#1] SMP NOPTI
>
> Reported-by: Marek Marczykowski-Górecki 
> Tested-by: Marek Marczykowski-Górecki 
> Fixes: 77c4adf6a6df ("xen/balloon: mark inflated pages PG_offline")
> Cc: sta...@vger.kernel.org # v5.1+
> Cc: Boris Ostrovsky 
> Cc: Juergen Gross 
> Cc: Stefano Stabellini 
> Signed-off-by: David Hildenbrand 
> ---
>  drivers/xen/balloon.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> index 05b1f7e948ef..29f6256363db 100644
> --- a/drivers/xen/balloon.c
> +++ b/drivers/xen/balloon.c
> @@ -687,6 +687,7 @@ static void __init balloon_add_region(unsigned long 
> start_pfn,
>   /* totalram_pages and totalhigh_pages do not
>  include the boot-time balloon extension, so
>  don't subtract from it. */
> + __SetPageOffline(page);
>   __balloon_append(page);


Given that when a page is appended to balloon list need to be marked
offline and, conversely, when a page is retrieved the bit should be
cleared I wonder whether it's better to move Set/ClearPageOffline to
balloon_append/retrieve().

-boris

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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf-pvops 6 kernel-build fail REGR. vs. 129313

Tests which are failing intermittently (not blocking):
 test-arm64-arm64-examine 11 examine-serial/bootloader  fail pass in 141813

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

version targeted for testing:
 linuxd573e8a79f70404ba08623d1de7ea617d55092ac
baseline version:
 linux84df9525b0c27f3ebc2ebb1864fa62a97fdedb7d

Last test of basis   129313  2018-11-02 05:39:08 Z  329 days
Failing since129412  2018-11-04 14:10:15 Z  327 days  245 attempts
Testing same since   141616  2019-09-22 03:05:43 Z5 days5 attempts


2592 people touched revisions under test,
not listing them all

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

Re: [Xen-devel] [PATCH RFC for-4.13 04/10] xen/arm: Ensure the SSBD workaround is re-enabled right after exiting a guest

[Volodymyr Babchuk]

Julien Grall writes:

> Hi,
>
> On 27/09/2019 15:21, Volodymyr Babchuk wrote:
>>
>> Julien Grall writes:
>>
>>> On 27/09/2019 14:33, Volodymyr Babchuk wrote:
 Julien Grall writes:
> On 27/09/2019 13:39, Volodymyr Babchuk wrote:
>> Julien Grall writes:
>>> On 27/09/2019 12:56, Volodymyr Babchuk wrote:
 Julien Grall writes:
>> Or maybe, we should not split the function at all? Instead, we enable
>> interrupts right in the middle of it.
>
> I thought about this but I didn't much like the resulting code.
>
> The instruction to unmask interrupts requires to take an immediate
> (indicates which interrupts to unmask). As not all the traps require
> to unmask the same interrupts, we would end up to have to a bunch of
> if in the code to select the right unmasking.
 Ah, yes, this is the problem. We can provide callback to
 enter_hypervisor_from_guest().
>>>
>>> I am not sure what you mean by this. Do you mean a callback that will
>>> unmask the interrupts?
>> Yes. You can pass function pointer to enter_hypervisor_from_guest(). To
>> a function, that will unmask the interrupts. I'm sure that guest_vector
>> macro can generate it for you. Something like this:
>>
>>  .macro  guest_vector compat, iflags, trap, save_x0_x1=1
>>  entry   hyp=0, compat=\compat, save_x0_x1=\save_x0_x1
>>  /*
>>   * The vSError will be checked while 
>> SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
>>   * is not set. If a vSError took place, the initial exception will 
>> be
>>   * skipped. Exit ASAP
>>   */
>>  ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
>>  "nop; nop",
>>  SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
>>  ldr x0, =1f
>>  bl  enter_hypervisor_from_guest
>>  mov x0, sp
>>  bl  do_trap_\trap
>>  b   1f
>> 2:
>>  msr daifclr, \iflags
>>  ret
>> 1:
>>  exithyp=0, compat=\compat
>>  .endm
>
> TBH, I don't see what's the point you are trying to make here. Yes,
> there are many way to write a code and possibility to make one
> function. You could also create a skeleton macro for
> enter_hypervisor_from_guest and generate N of them (one per set of
> unmask interrupts) and call them.
>
> But are they really worth it?
The point is that you are trying to split one entity into two.
As I see it: semantically we have one function:
enter_hypervisor_from_guest(). The purpose of this function is clear:
finish transition from guest mode to hypervisor mode.

But because of some architectural limitation (daifclr requires only
immediate argument) you are forced to divide this function in two.
I don't like this, because entry path now is more complex. To follow
what is going you need to bounce between head.S and traps.c one extra time.

Anyways, this is only my opinion. I'm not forcing you to implement
enter_hypervisor_from_guest() in this way.

I'm okay with enter_hypervisor_from_guest_preirq() as well.

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

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

[osstest service owner]

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

Regressions :-(

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

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

Re: [Xen-devel] [PATCH v1] libxl: fix crash in helper_done due to uninitialized data

[Ian Jackson]

Olaf Hering writes ("[PATCH v1] libxl: fix crash in helper_done due to 
uninitialized data"):
> A crash in helper_done, called from libxl_domain_suspend, was reported,
> triggered by 'virsh migrate --live xen+ssh://host':
...
> This is triggered by a failed poll, the actual error was:
> 
> libxl_aoutils.c:328:datacopier_writable: unexpected poll event 0x1c on fd 37 
> (should be POLLOUT) writing libxc header during copy of save v2 stream
> 
> In this case revents in datacopier_writable is POLLHUP|POLLERR|POLLOUT,
> which triggers datacopier_callback. In helper_done,
> shs->completion_callback is still zero. libxl__xc_domain_save fills
> dss.sws.shs. But that function is only called after stream_header_done.
> Any error before that will leave dss partly uninitialized.

Thanks for the diagnosis.  And sorry for the inconvenience of this
bug.

> Fix this crash by checking if ->completion_callback is valid.

But I don't think this fix is right.  The bug is that
libxl__save_helper_abort is called on a blank shs.  (You say
"uninitialised" but actually this is all zeroed at some point, so it
it's not reading uninitialised memory.)

I think it is in fact a bug that this error path

if (!stream->rc && rc) {
/* First reported failure. Tear everything down. */
stream->rc = rc;
stream->sync_teardown = true;

libxl__stream_read_abort(egc, stream, rc);
libxl__save_helper_abort(egc, >shs);
libxl__conversion_helper_abort(egc, >chs, rc);

stream->sync_teardown = false;
}

calls libxl__save_helper_abort when it hasn't ever called anything
other than libxl__save_helper_init.  Because _abort naturally wants to
call the failure callback.

I suggest that we add a check for _inuse to this bit of
check_all_finished.

Ross and Andrew, you wrote much of this stream read stuff, what do you
think ?

Part of the difficulty is that the possible states and transitions of
a libxl__save_helper_state are not formatlly documented.  That's my
fault - sorry.

Thanks,
Ian.

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

Re: [Xen-devel] [PATCH v4 00/46] xen: add core scheduling support

[Dario Faggioli]

On Fri, 2019-09-27 at 09:00 +0200, Juergen Gross wrote:
> Add support for core- and socket-scheduling in the Xen hypervisor.
> 
Ok, I think I've reviewed all the patches. They all should have the
appropriate tag for going in (unless I've missed any) except patch 35,
for which I've asked more comments this morning.

If the patch have to go in by today, I guess I'm fine with that patch
going in as it is (i.e., committers can add my `Reviewed-by:`), as far
as the additional comments come in a follow-up patch as soon as
possible.

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



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

Re: [Xen-devel] [PATCH RFC for-4.13 04/10] xen/arm: Ensure the SSBD workaround is re-enabled right after exiting a guest

[Julien Grall]

Hi,

On 27/09/2019 15:21, Volodymyr Babchuk wrote:


Julien Grall writes:


On 27/09/2019 14:33, Volodymyr Babchuk wrote:

Julien Grall writes:

On 27/09/2019 13:39, Volodymyr Babchuk wrote:

Julien Grall writes:

On 27/09/2019 12:56, Volodymyr Babchuk wrote:

Julien Grall writes:

Or maybe, we should not split the function at all? Instead, we enable
interrupts right in the middle of it.


I thought about this but I didn't much like the resulting code.

The instruction to unmask interrupts requires to take an immediate
(indicates which interrupts to unmask). As not all the traps require
to unmask the same interrupts, we would end up to have to a bunch of
if in the code to select the right unmasking.

Ah, yes, this is the problem. We can provide callback to
enter_hypervisor_from_guest().


I am not sure what you mean by this. Do you mean a callback that will
unmask the interrupts?

Yes. You can pass function pointer to enter_hypervisor_from_guest(). To
a function, that will unmask the interrupts. I'm sure that guest_vector
macro can generate it for you. Something like this:

 .macro  guest_vector compat, iflags, trap, save_x0_x1=1
 entry   hyp=0, compat=\compat, save_x0_x1=\save_x0_x1
 /*
  * The vSError will be checked while SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
  * is not set. If a vSError took place, the initial exception will be
  * skipped. Exit ASAP
  */
 ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
 "nop; nop",
 SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
 ldr x0, =1f
 bl  enter_hypervisor_from_guest
 mov x0, sp
 bl  do_trap_\trap
 b   1f
2:
 msr daifclr, \iflags
 ret
1:
 exithyp=0, compat=\compat
 .endm


TBH, I don't see what's the point you are trying to make here. Yes, there are 
many way to write a code and possibility to make one function. You could also 
create a skeleton macro for enter_hypervisor_from_guest and generate N of them 
(one per set of unmask interrupts) and call them.


But are they really worth it?

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v4 44/46] xen/sched: support core scheduling for moving cpus to/from cpupools

[Dario Faggioli]

On Fri, 2019-09-27 at 09:00 +0200, Juergen Gross wrote:
> With core scheduling active it is necessary to move multiple cpus at
> the same time to or from a cpupool in order to avoid split scheduling
> resources in between.
> 
> Signed-off-by: Juergen Gross 
>
Reviewed-by: Dario Faggioli 

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



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

[Xen-devel] [PATCH v1] libxl: fix crash in helper_done due to uninitialized data

[Olaf Hering]

A crash in helper_done, called from libxl_domain_suspend, was reported,
triggered by 'virsh migrate --live xen+ssh://host':

 #1 helper_done (...) at libxl_save_callout.c:371
  helper_failed
  helper_stop
  libxl__save_helper_abort
 #2 check_all_finished (..., rc=-3) at libxl_stream_write.c:671
  stream_done
  stream_complete
  write_done
  dc->callback == write_done
  efd->func == datacopier_writable
 #3 afterpoll_internal (...) at libxl_event.c:1269

This is triggered by a failed poll, the actual error was:

libxl_aoutils.c:328:datacopier_writable: unexpected poll event 0x1c on fd 37 
(should be POLLOUT) writing libxc header during copy of save v2 stream

In this case revents in datacopier_writable is POLLHUP|POLLERR|POLLOUT,
which triggers datacopier_callback. In helper_done,
shs->completion_callback is still zero. libxl__xc_domain_save fills
dss.sws.shs. But that function is only called after stream_header_done.
Any error before that will leave dss partly uninitialized.

Fix this crash by checking if ->completion_callback is valid.

Signed-off-by: Olaf Hering 
---
 tools/libxl/libxl_save_callout.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_save_callout.c b/tools/libxl/libxl_save_callout.c
index 6452d70036..89a2f6ecf0 100644
--- a/tools/libxl/libxl_save_callout.c
+++ b/tools/libxl/libxl_save_callout.c
@@ -366,8 +366,9 @@ static void helper_done(libxl__egc *egc, 
libxl__save_helper_state *shs)
 assert(!libxl__save_helper_inuse(shs));
 
 shs->egc = egc;
-shs->completion_callback(egc, shs->caller_state,
- shs->rc, shs->retval, shs->errnoval);
+if (shs->completion_callback)
+shs->completion_callback(egc, shs->caller_state,
+ shs->rc, shs->retval, shs->errnoval);
 shs->egc = 0;
 }
 

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

Re: [Xen-devel] [PATCH v4 43/46] xen/sched: support differing granularity in schedule_cpu_[add/rm]()

[Dario Faggioli]

On Fri, 2019-09-27 at 09:00 +0200, Juergen Gross wrote:
> With core scheduling active schedule_cpu_[add/rm]() has to cope with
> different scheduling granularity: a cpu not in any cpupool is subject
> to granularity 1 (cpu scheduling), while a cpu in a cpupool might be
> in a scheduling resource with more than one cpu.
> 
> Handle that by having arrays of old/new pdata and vdata and loop over
> those where appropriate.
> 
> Additionally the scheduling resource(s) must either be merged or
> split.
> 
> Signed-off-by: Juergen Gross 
>
Reviewed-by: Dario Faggioli 

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



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

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

[osstest service owner]

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

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  fab256ba5feee451a07c6cb4fe3ec12d2d7a5b41
baseline version:
 xen  ddc5a85fbcfbacc34bbd9abcdb12923de2fc27b3

Last test of basis   141875  2019-09-26 18:00:52 Z0 days
Testing same since   141902  2019-09-27 13:00:45 Z0 days1 attempts


People who touched revisions under test:
  Chao Gao 
  Juergen Gross 
  Oleksandr Tyshchenko 
  Paul Durrant 
  Roger Pau Monné 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  pass
 test-amd64-amd64-xl-qemuu-debianhvm-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
   ddc5a85fbc..fab256ba5f  fab256ba5feee451a07c6cb4fe3ec12d2d7a5b41 -> smoke

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

[Xen-devel] [PATCH v12] microcode: rendezvous CPUs in NMI handler and load ucode

[Chao Gao]

When one core is loading ucode, handling NMI on sibling threads or
on other cores in the system might be problematic. By rendezvousing
all CPUs in NMI handler, it prevents NMI acceptance during ucode
loading.

Basically, some work previously done in stop_machine context is
moved to NMI handler. Primary threads call in and load ucode in
NMI handler. Secondary threads wait for the completion of ucode
loading on all CPU cores. An option is introduced to disable this
behavior.

Control thread doesn't rendezvous in NMI handler by calling self_nmi()
(in case of unknown_nmi_error() being triggered). The side effect is
control thread might be handling an NMI while other threads are loading
ucode. If an ucode is to update something shared by a whole socket,
control thread may be accessing things that are being updating by the
ucode loading on other cores. It is not safe. Update ucode on the
control thread first to mitigate this issue.

Signed-off-by: Sergey Dyasli 
Signed-off-by: Chao Gao 
---
Note:
I plan to finish remaining patches (like handling parked CPU,
BDF90 and WBINVD, IMO, not important as this one) in RCs.
So this v12 only has one patch.

Changes in v12:
 - take care that self NMI may not arrive synchronously.
 - explain why control thread loads ucode first in patch description.
 - use parse_boolean to parse "scan" field in "ucode" option. The change
 is compatible with the old style.
 - staticify loading_err
 - drop primary_nmi_work()

Changes in v11:
 - Extend existing 'nmi' option rather than use a new one.
 - use per-cpu variable to store error code of xxx_nmi_work()
 - rename secondary_thread_work to secondary_nmi_work.
 - intialize nmi_patch to ZERO_BLOCK_PTR and make it static.
 - constify nmi_cpu
 - explain why control thread loads ucode first in patch description

Changes in v10:
 - rewrite based on Sergey's idea and patch
 - add Sergey's SOB.
 - add an option to disable ucode loading in NMI handler
 - don't send IPI NMI to the control thread to avoid unknown_nmi_error()
 in do_nmi().
 - add an assertion to make sure the cpu chosen to handle platform NMI
 won't send self NMI. Otherwise, there is a risk that we encounter
 unknown_nmi_error() and system crashes.

Changes in v9:
 - control threads send NMI to all other threads. Slave threads will
 stay in the NMI handling to prevent NMI acceptance during ucode
 loading. Note that self-nmi is invalid according to SDM.
 - s/rep_nop/cpu_relax
 - remove debug message in microcode_nmi_callback(). Printing debug
 message would take long times and control thread may timeout.
 - rebase and fix conflicts

Changes in v8:
 - new
---
 docs/misc/xen-command-line.pandoc |   6 +-
 xen/arch/x86/microcode.c  | 174 +++---
 xen/arch/x86/traps.c  |   6 +-
 xen/include/asm-x86/nmi.h |   3 +
 4 files changed, 156 insertions(+), 33 deletions(-)

diff --git a/docs/misc/xen-command-line.pandoc 
b/docs/misc/xen-command-line.pandoc
index fc64429..f5410b3 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -2053,7 +2053,7 @@ pages) must also be specified via the tbuf_size parameter.
 > `= unstable | skewed | stable:socket`
 
 ### ucode (x86)
-> `= [ | scan]`
+> `= List of [  | scan=, nmi= ]`
 
 Specify how and where to find CPU microcode update blob.
 
@@ -2074,6 +2074,10 @@ microcode in the cpio name space must be:
   - on Intel: kernel/x86/microcode/GenuineIntel.bin
   - on AMD  : kernel/x86/microcode/AuthenticAMD.bin
 
+'nmi' determines late loading is performed in NMI handler or just in
+stop_machine context. In NMI handler, even NMIs are blocked, which is
+considered safer. The default value is `true`.
+
 ### unrestricted_guest (Intel)
 > `= `
 
diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
index b882ac8..3c0f72e 100644
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode.c
@@ -36,8 +36,10 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -95,6 +97,9 @@ static struct ucode_mod_blob __initdata ucode_blob;
  */
 static bool_t __initdata ucode_scan;
 
+/* By default, ucode loading is done in NMI handler */
+static bool ucode_in_nmi = true;
+
 /* Protected by microcode_mutex */
 static struct microcode_patch *microcode_cache;
 
@@ -105,23 +110,40 @@ void __init microcode_set_module(unsigned int idx)
 }
 
 /*
- * The format is '[|scan]'. Both options are optional.
- * If the EFI has forced which of the multiboot payloads is to be used,
- * no parsing will be attempted.
+ * The format is '[|scan=, nmi=]'. Both options are
+ * optional. If the EFI has forced which of the multiboot payloads is to be
+ * used, only nmi= is parsed.
  */
 static int __init parse_ucode(const char *s)
 {
-const char *q = NULL;
+const char *ss;
+int val, rc = 0;
 
-if ( ucode_mod_forced ) /* Forced by EFI */
-   return 0;
+do {
+ss = strchr(s, ',');
+if ( !ss )
+ss = 

Re: [Xen-devel] [PATCH v4 42/46] xen/sched: support multiple cpus per scheduling resource

[Dario Faggioli]

On Fri, 2019-09-27 at 09:00 +0200, Juergen Gross wrote:
> Prepare supporting multiple cpus per scheduling resource by
> allocating
> the cpumask per resource dynamically.
> 
> Modify sched_res_mask to have only one bit per scheduling resource
> set.
> 
> Signed-off-by: Juergen Gross 
>
Reviewed-by: Dario Faggioli 

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



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

[Xen-devel] [PATCH] xen/efi: Set nonblocking callbacks

[Ross Lagerwall]

Other parts of the kernel expect these nonblocking EFI callbacks to
exist and crash when running under Xen. Since the implementations of
xen_efi_set_variable() and xen_efi_query_variable_info() do not take any
locks, use them for the nonblocking callbacks too.

Signed-off-by: Ross Lagerwall 
---
 arch/arm/xen/efi.c | 2 ++
 arch/x86/xen/efi.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/arch/arm/xen/efi.c b/arch/arm/xen/efi.c
index d687a73044bf..cb2aaf98e243 100644
--- a/arch/arm/xen/efi.c
+++ b/arch/arm/xen/efi.c
@@ -19,7 +19,9 @@ void __init xen_efi_runtime_setup(void)
efi.get_variable = xen_efi_get_variable;
efi.get_next_variable= xen_efi_get_next_variable;
efi.set_variable = xen_efi_set_variable;
+   efi.set_variable_nonblocking = xen_efi_set_variable;
efi.query_variable_info  = xen_efi_query_variable_info;
+   efi.query_variable_info_nonblocking = xen_efi_query_variable_info;
efi.update_capsule   = xen_efi_update_capsule;
efi.query_capsule_caps   = xen_efi_query_capsule_caps;
efi.get_next_high_mono_count = xen_efi_get_next_high_mono_count;
diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index 0d3365cb64de..7e3eb70f411a 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -65,7 +65,9 @@ static efi_system_table_t __init *xen_efi_probe(void)
efi.get_variable = xen_efi_get_variable;
efi.get_next_variable= xen_efi_get_next_variable;
efi.set_variable = xen_efi_set_variable;
+   efi.set_variable_nonblocking = xen_efi_set_variable;
efi.query_variable_info  = xen_efi_query_variable_info;
+   efi.query_variable_info_nonblocking = xen_efi_query_variable_info;
efi.update_capsule   = xen_efi_update_capsule;
efi.query_capsule_caps   = xen_efi_query_capsule_caps;
efi.get_next_high_mono_count = xen_efi_get_next_high_mono_count;
-- 
2.21.0


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

[Xen-devel] [PATCH v1] xen/balloon: Set pages PageOffline() in balloon_add_region()

[David Hildenbrand]

We are missing a __SetPageOffline(), which is why we can get
!PageOffline() pages onto the balloon list, where
alloc_xenballooned_pages() will complain:

page:ea0003e7ffc0 refcount:1 mapcount:0 mapping: index:0x0
flags: 0xe1000(reserved)
raw: 000e1000 dead0100 dead0200 
raw:   0001 
page dumped because: VM_BUG_ON_PAGE(!PageOffline(page))
[ cut here ]
kernel BUG at include/linux/page-flags.h:744!
invalid opcode:  [#1] SMP NOPTI

Reported-by: Marek Marczykowski-Górecki 
Tested-by: Marek Marczykowski-Górecki 
Fixes: 77c4adf6a6df ("xen/balloon: mark inflated pages PG_offline")
Cc: sta...@vger.kernel.org # v5.1+
Cc: Boris Ostrovsky 
Cc: Juergen Gross 
Cc: Stefano Stabellini 
Signed-off-by: David Hildenbrand 
---
 drivers/xen/balloon.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 05b1f7e948ef..29f6256363db 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -687,6 +687,7 @@ static void __init balloon_add_region(unsigned long 
start_pfn,
/* totalram_pages and totalhigh_pages do not
   include the boot-time balloon extension, so
   don't subtract from it. */
+   __SetPageOffline(page);
__balloon_append(page);
}
 
-- 
2.21.0


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

Re: [Xen-devel] [PATCH v4 41/46] xen/sched: protect scheduling resource via rcu

[Dario Faggioli]

On Fri, 2019-09-27 at 09:00 +0200, Juergen Gross wrote:
> In order to be able to move cpus to cpupools with core scheduling
> active it is mandatory to merge multiple cpus into one scheduling
> resource or to split a scheduling resource with multiple cpus in it
> into multiple scheduling resources. This in turn requires to modify
> the cpu <-> scheduling resource relation. In order to be able to free
> unused resources protect struct sched_resource via RCU. This ensures
> there are no users left when freeing such a resource.
> 
> Signed-off-by: Juergen Gross 
>
Reviewed-by: Dario Faggioli 

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



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

Re: [Xen-devel] [PATCH v4 40/46] xen/sched: split schedule_cpu_switch()

[Dario Faggioli]

On Fri, 2019-09-27 at 09:00 +0200, Juergen Gross wrote:
> Instead of letting schedule_cpu_switch() handle moving cpus from and
> to cpupools, split it into schedule_cpu_add() and schedule_cpu_rm().
> 
> This will allow us to drop allocating/freeing scheduler data for free
> cpus as the idle scheduler doesn't need such data.
> 
> Signed-off-by: Juergen Gross 
>
Reviewed-by: Dario Faggioli 

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



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

Re: [Xen-devel] [PATCH RFC for-4.13 08/10] xen/arm: alternative: add auto-nop infrastructure

[Volodymyr Babchuk]

Julien Grall writes:

> From: Mark Rutland 
>
> In some cases, one side of an alternative sequence is simply a number of
> NOPs used to balance the other side. Keeping track of this manually is
> tedious, and the presence of large chains of NOPs makes the code more
> painful to read than necessary.
>
> To ameliorate matters, this patch adds a new alternative_else_nop_endif,
> which automatically balances an alternative sequence with a trivial NOP
> sled.
>
> In many cases, we would like a NOP-sled in the default case, and
> instructions patched in in the presence of a feature. To enable the NOPs
> to be generated automatically for this case, this patch also adds a new
> alternative_if, and updates alternative_else and alternative_endif to
> work with either alternative_if or alternative_endif.
>
> The alternative infrastructure was originally ported from Linux. So this
> is pretty much a straight backport from commit 792d47379f4d "arm64:
> alternative: add auto-nop infrastructure". The only difference is the
> nops macro added as not yet existing in Xen.
>
> Signed-off-by: Mark Rutland 
> [will: use new nops macro to generate nop sequences]
> Signed-off-by: Will Deacon 
> [julien: Add nops and port to Xen]
> Signed-off-by: Julien Grall 
Reviewed-by: Volodymyr Babchuk 


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

Re: [Xen-devel] VM_BUG_ON_PAGE(!PageOffline(page), page) in alloc_xenballooned_pages

[Marek Marczykowski-Górecki]

On Fri, Sep 27, 2019 at 09:44:35AM +0200, David Hildenbrand wrote:
> On 26.09.19 23:34, Marek Marczykowski-Górecki wrote:
> > Hi,
> > 
> > I've hit VM_BUG_ON_PAGE(!PageOffline(page), page) in
> > alloc_xenballooned_pages, when trying to use gnttab from userspace
> > application. It happens on Xen PV, but not on Xen PVH or HVM with the
> > same kernel. This happens at least with 5.1.6, but also 5.2.15
> > (as seen below). Based on this, it looks related to 0266def91377
> > (xen/balloon: Fix mapping PG_offline pages to user space) and probably
> > 77c4adf6a6df (xen/balloon: mark inflated pages PG_offline).
> > 
> > Any idea? Below is full message.
> 
> Now that's weird. Weird because half a year passed since 
> 0266def91377 (xen/balloon: Fix mapping PG_offline pages to user space).

Not sure about others, but in Qubes we use PVH/HVM VMs mostly.

> > page:ea0003e7ffc0 refcount:1 mapcount:0 mapping: 
> > index:0x0
> > flags: 0xe1000(reserved)
> 
> So we have a PageReserved page that is not PageOffline. I assume this
> happens when we do a __ClearPageOffline() in alloc_xenballooned_pages().
> 
> That means, that we get such a page via balloon_retrieve(true). Which
> means that we have such a page sitting in the ballooned_pages list, which
> is weird.
> 
> Pages enter ballooned_pages via __balloon_append() only.
> 
> 1. Via xen_online_page(). We have a __SetPageOffline() right in front
>of it.
> 2. Via balloon_add_region(). I don't see a __SetPageOffline().
> 3. Via decrease_reservation(). We seem to do a __SetPageOffline() on all
>pages in the previous loop.
> 4. Via free_xenballooned_pages(). We have a __SetPageOffline() right
>in front of it.
> 
> 
> So this smells like #2 (and matches your PV only observation). Also,
> it makes sense that the page is PageReserved that way.
> 
> 
> Wonder if it is as easy as:

Yes, besides missing semicolon it works. Thanks!

> From 0955beef5aa11da4a8398472ce3106a92599cbe6 Mon Sep 17 00:00:00 2001
> From: David Hildenbrand 
> Date: Fri, 27 Sep 2019 09:39:31 +0200
> Subject: [PATCH v1] xen/balloon: Set pages PageOffline() in
>  balloon_add_region()
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> We are missing a __SetPageOffline(), which is why we can get
> !PageOffline() pages onto the balloon list, where
> alloc_xenballooned_pages() will complain:
> 
> page:ea0003e7ffc0 refcount:1 mapcount:0 mapping: index:0x0
> flags: 0xe1000(reserved)
> raw: 000e1000 dead0100 dead0200 
> raw:   0001 
> page dumped because: VM_BUG_ON_PAGE(!PageOffline(page))
> [ cut here ]
> kernel BUG at include/linux/page-flags.h:744!
> invalid opcode:  [#1] SMP NOPTI
> 
> Reported-by: Marek Marczykowski-Górecki 
> Fixes: 77c4adf6a6df ("xen/balloon: mark inflated pages PG_offline")
> Cc: sta...@vger.kernel.org # v5.1+
> Signed-off-by: David Hildenbrand 

Tested-by: Marek Marczykowski-Górecki 

> ---
>  drivers/xen/balloon.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> index 05b1f7e948ef..d31149068448 100644
> --- a/drivers/xen/balloon.c
> +++ b/drivers/xen/balloon.c
> @@ -687,6 +687,7 @@ static void __init balloon_add_region(unsigned long 
> start_pfn,
>   /* totalram_pages and totalhigh_pages do not
>  include the boot-time balloon extension, so
>  don't subtract from it. */
> + __SetPageOffline(page)
  ^
  ;

>   __balloon_append(page);
>   }
>  

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


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

Re: [Xen-devel] [PATCH v4 39/46] xen/sched: prepare per-cpupool scheduling granularity

[Dario Faggioli]

On Fri, 2019-09-27 at 09:00 +0200, Juergen Gross wrote:
> On- and offlining cpus with core scheduling is rather complicated as
> the cpus are taken on- or offline one by one, but scheduling wants
> them
> rather to be handled per core.
> 
> As the future plan is to be able to select scheduling granularity per
> cpupool prepare that by storing the granularity in struct cpupool and
> struct sched_resource (we need it there for free cpus which are not
> associated to any cpupool). Free cpus will always use granularity 1.
> 
> Store the selected granularity option (cpu, core or socket) in the
> cpupool as well, as we will need it to select the appropriate cpu
> mask
> when populating the cpupool with cpus.
> 
> This will make on- and offlining of cpus much easier and avoids
> writing code which would needed to be thrown away later.
> 
> Move the granularity related variables to cpupool.c as they are now
> used form there only.
> 
> Signed-off-by: Juergen Gross 
>
Reviewed-by: Dario Faggioli 

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



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

Re: [Xen-devel] [PATCH RFC for-4.13 10/10] xen/arm64: entry: Ensure the guest state is synced when receiving a vSError

[Volodymyr Babchuk]

Hi Julien,

Julien Grall writes:

> At the moment, when a SError is received while checking for a pending
> one, we will skip the handling the initial exception.
>
> This includes call to exit_from_guest{, _noirq} that is used to
Did you mean enter_hypervisor_from_guest{, _noirq} there? Otherwise, I'm
confused a lot.

> synchronize part of the guest state with the internal representation.
> However, we still call leave_hypervisor_tail() which is used for preempting
> the guest and synchronizing back part of the guest state.
>
> exit_from_guest{, _noirq} works in pair with leave_hypervisor_tail(), so
> skipping if former may result to a loss of some part of  guest state.
> An example is the new vGIC which will save the state of the LRS on exit
> from the guest and rewrite all of them on entry to the guest.
>
> For now, calling leave_hypervisor_tail() is not necessary when injecting
> a vSError to the guest. But as the path is spread accross multiple file,
> it is hard to enforce that for the future (someone we may want to crash the
> domain). Therefore it is best to call exit_from_guest{, _noirq} in the
> vSError path as well.
>
> Note that the return value of check_pending_vserror is now set in x19
> instead of x0. This is because we want to keep the value across call to
> C-function and x0, unlike x19, will not be saved by the callee.
>
> Signed-off-by: Julien Grall 
>
> ---
>
> I am not aware of any issues other than with the new vGIC. But I
> haven't looked hard enough so I think it would be worth to try to fix it
> for Xen 4.13.
> ---
>  xen/arch/arm/arm64/entry.S | 21 ++---
>  1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
> index 91cf6ee6f4..f5350247e1 100644
> --- a/xen/arch/arm/arm64/entry.S
> +++ b/xen/arch/arm/arm64/entry.S
> @@ -168,11 +168,13 @@
>  /*
>   * The vSError will be checked while 
> SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
>   * is not set. If a vSError took place, the initial exception will be
> - * skipped. Exit ASAP
> + * skipped.
> + *
> + * However, we still need to call exit_from_guest{,_noirq} as the
> + * return path to the guest may rely on state saved by them.
>   */
>  alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
>  bl  check_pending_vserror
> -cbnzx0, 1f
>  alternative_else_nop_endif
>  
>  mov x0, sp
> @@ -180,6 +182,11 @@
>  msr daifclr, \iflags
>  mov x0, sp
>  bl  enter_hypervisor_from_guest
> +
> +alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
> +cbnzx19, 1f
> +alternative_else_nop_endif
> +
>  mov x0, sp
>  bl  do_trap_\trap
>  1:
> @@ -383,9 +390,9 @@ return_from_trap:
>  /*
>   * This function is used to check pending virtual SError in the gap of
>   * EL1 -> EL2 world switch.
> - * The x0 register will be used to indicate the results of detection.
> - * x0 -- Non-zero indicates a pending virtual SError took place.
> - * x0 -- Zero indicates no pending virtual SError took place.
> + * The register x19 will be used to indicate the results of detection.
> + * x19 -- Non-zero indicates a pending virtual SError took place.
> + * x19 -- Zero indicates no pending virtual SError took place.
>   */
>  check_pending_vserror:
>  /*
> @@ -432,9 +439,9 @@ abort_guest_exit_end:
>  
>  /*
>   * Not equal, the pending SError exception took place, set
> - * x0 to non-zero.
> + * x19 to non-zero.
>   */
> -csetx0, ne
> +csetx19, ne
>  
>  ret


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

Re: [Xen-devel] [PATCH v4 35/46] xen/sched: make vcpu_wake() and vcpu_sleep() core scheduling aware

[Dario Faggioli]

On Fri, 2019-09-27 at 09:00 +0200, Juergen Gross wrote:
> vcpu_wake() and vcpu_sleep() need to be made core scheduling aware:
> they might need to switch a single vcpu of an already scheduled unit
> between running and not running.
> 
> Especially when vcpu_sleep() for a vcpu is being called by a vcpu of
> the same scheduling unit special care must be taken in order to avoid
> a deadlock: the vcpu to be put asleep must be forced through a
> context switch without doing so for the calling vcpu. For this
> purpose add a vcpu flag handled in sched_slave() and in
> sched_wait_rendezvous_in() allowing a vcpu of the currently running
> unit to switch state at a higher priority than a normal schedule
> event.
> 
> Use the same mechanism when waking up a vcpu of a currently active
> unit.
> 
> While at it make vcpu_sleep_nosync_locked() static as it is used in
> schedule.c only.
> 
> Signed-off-by: Juergen Gross 
>
I'm ok with the code in this patch.

I'd just like to see the comment(s) around the asymmetry between
vcpu_sleep_xxx() and vcpu_wake() added, as agreed upon this morning (in
the V3 thread).

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



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

Re: [Xen-devel] [PATCH v4 34/46] xen/sched: add fall back to idle vcpu when scheduling unit

[Dario Faggioli]

On Fri, 2019-09-27 at 09:00 +0200, Juergen Gross wrote:
> When scheduling an unit with multiple vcpus there is no guarantee all
> vcpus are available (e.g. above maxvcpus or vcpu offline). Fall back
> to
> idle vcpu of the current cpu in that case. This requires to store the
> correct schedule_unit pointer in the idle vcpu as long as it used as
> fallback vcpu.
> 
> In order to modify the runstates of the correct vcpus when switching
> schedule units merge sched_unit_runstate_change() into
> sched_switch_units() and loop over the affected physical cpus instead
> of the unit's vcpus. This in turn requires an access function to the
> current variable of other cpus.
> 
> Today context_saved() is called in case previous and next vcpus
> differ
> when doing a context switch. With an idle vcpu being capable to be a
> substitute for an offline vcpu this is problematic when switching to
> an idle scheduling unit. An idle previous vcpu leaves us in doubt
> which
> schedule unit was active previously, so save the previous unit
> pointer
> in the per-schedule resource area. If it is NULL the unit has not
> changed and we don't have to set the previous unit to be not running.
> 
> When running an idle vcpu in a non-idle scheduling unit use a
> specific
> guest idle loop not performing any non-softirq tasklets and
> livepatching in order to avoid populating the cpu caches with memory
> used by other domains (as far as possible). Softirqs are considered
> to
> be save.
> 
> In order to avoid livepatching when going to guest idle another
> variant of reset_stack_and_jump() not calling
> check_for_livepatch_work
> is needed.
> 
> Signed-off-by: Juergen Gross 
> Acked-by: Julien Grall 
>
Reviewed-by: Dario Faggioli 

With one request.

> @@ -279,21 +293,11 @@ static inline void vcpu_runstate_change(
>  v->runstate.state = new_state;
>  }
>  
> -static inline void sched_unit_runstate_change(struct sched_unit
> *unit,
> -bool running, s_time_t new_entry_time)
> +void sched_guest_idle(void (*idle) (void), unsigned int cpu)
>  {
> -struct vcpu *v;
> -
> -for_each_sched_unit_vcpu ( unit, v )
> -{
> -if ( running )
> -vcpu_runstate_change(v, v->new_state, new_entry_time);
> -else
> -vcpu_runstate_change(v,
> -((v->pause_flags & VPF_blocked) ? RUNSTATE_blocked :
> - (vcpu_runnable(v) ? RUNSTATE_runnable :
> RUNSTATE_offline)),
> -new_entry_time);
> -}
> +atomic_inc(_cpu(sched_urgent_count, cpu));
> +idle();
> +atomic_dec(_cpu(sched_urgent_count, cpu));
>  }
>  
So, I recall that during review of v1, there was a discussion about why
we were marking this as urgent. You said it was to avoid latencies,
which makes sense. Jan said this is a policy that should be set
elsewhere, which also makes sense.

Ideally, we'd check with specific tests and benchmark whether playing
with the urgent counter in here is really necessary/good. That was, in
fact, my plan, but I did not got round to actually doing that.

So, can we have a comment stating the reason why we're doing this? I'd
like for that information not to be lost in a random email thread on
xen-devel. And that would also remind us (me? :-P) to actually go and
check things with actual benchmarks, at some point.

I'd be fine if such comment would come from a follow-up patch, (as,
since it will only add comments, I expect it can go in during the
freeze).

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



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

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

[osstest service owner]

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 6da405ebf6e946429953c56fcbc8f60abe1407de
baseline version:
 ovmf c70fef962e804eba483512b64ec24169871060be

Last test of basis   141845  2019-09-26 03:19:02 Z1 days
Testing same since   141888  2019-09-27 05:42:59 Z0 days1 attempts


People who touched revisions under test:
  Fan, ZhijuX 
  Zhiju.Fan 

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
   c70fef962e..6da405ebf6  6da405ebf6e946429953c56fcbc8f60abe1407de -> 
xen-tested-master

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

Re: [Xen-devel] [PATCH v1 1/6] docs: add feature document for Xen hypervisor sysfs-like support

[Ian Jackson]

Jürgen Groß writes ("Re: [PATCH v1 1/6] docs: add feature document for Xen 
hypervisor sysfs-like support"):
> On 27.09.19 15:37, Ian Jackson wrote:
> > I think this is ASCII printing characters with the exception of
> >! " ` $ % ^ & * = + { } ' ~ < > / \ |
> > 
> > I struggle to find a principled explanation for this particular
> > exclusion set (apart from /), considering that following are
> > included:
> >- @ _ . : ( ) [ ] # , ;
> > 
> > Could we borrow some existing permitted character set ?  If we are
> > permitting shell metacharacters why not just permit all printable
> > ASCII except / ?
> 
> Hmm, maybe we should allow just the "Posix portable file name character
> set"? That would be [-._0-9A-Za-z]. And we should explicitly not allow
> the key names "." and "..".

I agree about "." and "..".

I'm not sure the "Posix portable file name character set" is a very
good guide.  Posix will be pretty restricted there.  What is the legal
set in a Linux sysfs filename ?

> > Do you mean "any 7-bit byte", or octet values 32-126 (0x20-0x7e)
> > inclusive, or something else ?
> 
> :-)
> 
> As I'd like to support e.g. the .config file contents of the hypervisor
> build I guess I need (0x01-0xff) inclusive, right?

The .config file I have here does not seem to contain any control
characters.  If it did surely that would be a bug?  If you want this
you obviously do need to permit newlines and spaces and perhaps tabs
too.

Ian.

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

Re: [Xen-devel] [PATCH v4 33/46] xen/sched: add a percpu resource index

[Dario Faggioli]

On Fri, 2019-09-27 at 09:00 +0200, Juergen Gross wrote:
> Add a percpu variable holding the index of the cpu in the current
> sched_resource structure. This index is used to get the correct vcpu
> of a sched_unit on a specific cpu.
> 
> For now this index will be zero for all cpus, but with core
> scheduling
> it will be possible to have higher values, too.
> 
> Signed-off-by: Juergen Gross 
>
Reviewed-by: Dario Faggioli 

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



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

Re: [Xen-devel] [RFC PATCH 71/84] x86/setup: start tearing down the direct map.

[hongyax]

On 27/09/2019 15:14, Jan Beulich wrote:

On 27.09.2019 16:02, hong...@amazon.com wrote:

But then why do the initial so many patches (inherited from Wei)
convert from domheap to xenheap allocations at all? If your
approach is to be at least an intermediate goal, then I think the
order of changes should be such that on-demand mapping of xenheap
pages occurs first, and then the xenheap -> domheap conversion
can happen in basically arbitrarily small steps.



Also I have tested Wei's patches with fixes. It is pretty stable against my 
setup because the direct map has not been actually removed. I am able to run 
XTF tests, boot dom0, launch, restart and destroy guests without breakage. From 
a stability point of view, it probably makes more sense for Wei's patches to go 
in first. From the reviews, it looks like my patches to actually remove the 
direct map can benefit from more RFCs, and can be separated from Wei's into a 
second batch.


Hongyan

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

Re: [Xen-devel] [PATCH v4 32/46] xen/sched: support allocating multiple vcpus into one sched unit

[Dario Faggioli]

On Fri, 2019-09-27 at 09:00 +0200, Juergen Gross wrote:
> With a scheduling granularity greater than 1 multiple vcpus share the
> same struct sched_unit. Support that.
> 
> Setting the initial processor must be done carefully: we can't use
> sched_set_res() as that relies on for_each_sched_unit_vcpu() which in
> turn needs the vcpu already as a member of the domain's vcpu linked
> list, which isn't the case.
> 
> Signed-off-by: Juergen Gross 
>
Reviewed-by: Dario Faggioli 

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



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

Re: [Xen-devel] [PATCH v1 1/6] docs: add feature document for Xen hypervisor sysfs-like support

[Jürgen Groß]

On 27.09.19 15:37, Ian Jackson wrote:

Jürgen Groß writes ("Re: [PATCH v1 1/6] docs: add feature document for Xen 
hypervisor sysfs-like support"):

On 27.09.19 12:37, Ian Jackson wrote:

I guess that keys will be chosen from some limited safe character
set ?  What about values ?  Might we create a key whose value contains
binary data ?


I'd go with "[-A-Za-z0-9@_.:()\[\]#,;]*" for keys


I think this is ASCII printing characters with the exception of
   ! " ` $ % ^ & * = + { } ' ~ < > / \ |

I struggle to find a principled explanation for this particular
exclusion set (apart from /), considering that following are
included:
   - @ _ . : ( ) [ ] # , ;

Could we borrow some existing permitted character set ?  If we are
permitting shell metacharacters why not just permit all printable
ASCII except / ?


Hmm, maybe we should allow just the "Posix portable file name character
set"? That would be [-._0-9A-Za-z]. And we should explicitly not allow
the key names "." and "..".




and ASCII for values.


Do you mean "any 7-bit byte", or octet values 32-126 (0x20-0x7e)
inclusive, or something else ?


:-)

As I'd like to support e.g. the .config file contents of the hypervisor
build I guess I need (0x01-0xff) inclusive, right?




Would it be possible to add a script to xen.git which lists the
xenhypfs and checks that all the paths are documented ?  We could add
a few calls to that to osstest...


I'd expect some parts to be described rather generically (as can be seen
in patch 6 for the runtime parameters). Maybe I should add the entries
with wildcards there.


That would be nice.


Okay, will do.


Juergen

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

Re: [Xen-devel] [PATCH] x86: correct bogus error indicator of cpu_add()

[Jürgen Groß]

On 27.09.19 16:43, Jan Beulich wrote:

Commit 54ce2db8b8 ("x86/numa: adjust datatypes for node and pxm")
changed this from the -1 (i.e. -EPERM, which was already bogus) that
comes back from setup_node() to NUMA_NO_NODE (0xff). Use a proper error
indicator instead.

Signed-off-by: Jan Beulich 


Release-acked-by: Juergen Gross 


Juergen

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

Re: [Xen-devel] [PATCH v4 30/46] xen/sched: add support for multiple vcpus per sched unit where missing

[Dario Faggioli]

On Fri, 2019-09-27 at 09:00 +0200, Juergen Gross wrote:
> In several places there is support for multiple vcpus per sched unit
> missing. Add that missing support (with the exception of initial
> allocation) and missing helpers for that.
> 
> Signed-off-by: Juergen Gross 
>
Reviewed-by: Dario Faggioli 

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



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

Re: [Xen-devel] [RFC PATCH 71/84] x86/setup: start tearing down the direct map.

[hongyax]

On 27/09/2019 15:14, Jan Beulich wrote:

On 27.09.2019 16:02, hong...@amazon.com wrote:

But then why do the initial so many patches (inherited from Wei)
convert from domheap to xenheap allocations at all? If your
approach is to be at least an intermediate goal, then I think the
order of changes should be such that on-demand mapping of xenheap
pages occurs first, and then the xenheap -> domheap conversion
can happen in basically arbitrarily small steps.


There is this problem that anything that maps/unmaps memory in the direct map 
region cannot itself rely on an always-mapped direct map. Unfortunately, if I 
map/unmap xenheap allocations, page table manipulation functions (like 
map_pages_to_xen, alloc_xen_pagetable) themselves rely on an always-mapped 
direct map, which often break if you leave holes in the direct map region. 
Wei's patches with some of my later patches break exactly this dependency, so 
page table manipulations themselves no longer rely on the direct map. Now we 
can actually start tearing down the direct map, including on-demand mapping of 
xenheap in the direct map region.


Hongyan

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

Re: [Xen-devel] [PATCH] x86: correct bogus error indicator of cpu_add()

[Andrew Cooper]

On 27/09/2019 15:43, Jan Beulich wrote:
> Commit 54ce2db8b8 ("x86/numa: adjust datatypes for node and pxm")
> changed this from the -1 (i.e. -EPERM, which was already bogus) that
> comes back from setup_node() to NUMA_NO_NODE (0xff). Use a proper error
> indicator instead.
>
> Signed-off-by: Jan Beulich 

Acked-by: Andrew Cooper 

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

[Xen-devel] [PATCH] x86: correct bogus error indicator of cpu_add()

[Jan Beulich]

Commit 54ce2db8b8 ("x86/numa: adjust datatypes for node and pxm")
changed this from the -1 (i.e. -EPERM, which was already bogus) that
comes back from setup_node() to NUMA_NO_NODE (0xff). Use a proper error
indicator instead.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -1281,7 +1281,7 @@ int cpu_add(uint32_t apic_id, uint32_t a
 "Setup node failed for pxm %x\n", pxm);
 x86_acpiid_to_apicid[acpi_id] = BAD_APICID;
 mp_unregister_lapic(apic_id, cpu);
-cpu = node;
+cpu = -ENOSPC;
 goto out;
 }
 if ( apic_id < MAX_LOCAL_APIC )

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

Re: [Xen-devel] [PATCH v5 5/8] xen/arm: assign devices to boot domains

[Oleksandr]

On 26.09.19 00:12, Julien Grall wrote:

Hi Stefano,



Hi Stefano, Julien




On 25/09/2019 19:49, Stefano Stabellini wrote:

Scan the user provided dtb fragment at boot. For each device node, map
memory to guests, and route interrupts and setup the iommu.

The memory region to remap is specified by the "xen,reg" property.

The iommu is setup by passing the node of the device to assign on the
host device tree. The path is specified in the device tree fragment as
the "xen,path" string property.

The interrupts are remapped based on the information from the
corresponding node on the host device tree. Call
handle_device_interrupts to remap interrupts. Interrupts related device
tree properties are copied from the device tree fragment, same as all
the other properties.

Also add the new flag XEN_DOMCTL_CDF_iommu to that dom0less domU can use
the IOMMU.

Signed-off-by: Stefano Stabellini 
---
Changes in v5:
- use local variable for name
- use map_regions_p2mt
- add warning for not page aligned addresses/sizes
- introduce handle_passthrough_prop

Changes in v4:
- use unsigned
- improve commit message
- code style
- use dt_prop_cmp
- use device_tree_get_reg
- don't copy over xen,reg and xen,path
- don't create special interrupt properties for domU: copy them from the
fragment
- in-code comment

Changes in v3:
- improve commit message
- remove superfluous cast
- merge code with the copy code
- add interrup-parent
- demove depth > 2 check
- reuse code from handle_device_interrupts
- copy interrupts from host dt

Changes in v2:
- rename "path" to "xen,path"
- grammar fix
- use gaddr_to_gfn and maddr_to_mfn
- remove depth <= 2 limitation in scanning the dtb fragment
- introduce and parse xen,reg
- code style
- support more than one interrupt per device
- specify only the GIC is supported
---
   xen/arch/arm/domain_build.c | 101 ++--
   1 file changed, 97 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 9d985d3bbe..414893bc24 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1701,6 +1701,85 @@ static int __init make_vpl011_uart_node(struct 
kernel_info *kinfo)
   }
   #endif
   
+/*

+ * Scan device tree properties for passthrough specific information.
+ * Returns -ENOENT when no passthrough properties are found
+ * < 0 on error
+ * 0 on success
+ */
+static int __init handle_passthrough_prop(struct kernel_info *kinfo,
+  const struct fdt_property *prop,
+  const char *name,
+  uint32_t address_cells, uint32_t 
size_cells)
+{
+const __be32 *cell;
+unsigned int i, len;
+struct dt_device_node *node;
+int res;
+
+/* xen,reg specifies where to map the MMIO region */
+if ( dt_prop_cmp("xen,reg", name) == 0 )
+{
+paddr_t mstart, size, gstart;
+cell = (const __be32 *)prop->data;
+len = fdt32_to_cpu(prop->len) /
+((address_cells * 2 + size_cells) * sizeof(uint32_t));
+
+for ( i = 0; i < len; i++ )
+{
+device_tree_get_reg(, address_cells, size_cells,
+, );
+gstart = dt_next_cell(address_cells, );
+
+if ( gstart & ~PAGE_MASK || mstart & ~PAGE_MASK || size & 
~PAGE_MASK )
+dprintk(XENLOG_WARNING,
+"DomU passthrough config has not page aligned 
addresses/sizes\n");

I don't think this is wise to continue, the more this is a printk that
can only happen in debug build. So someone using a release build may not
notice the error.

So I think this wants to be a printk(XENLOG_ERR,...) and also return an
error.


+
+res = map_regions_p2mt(kinfo->d,
+gaddr_to_gfn(gstart),
+PFN_DOWN(size),
+maddr_to_mfn(mstart),
+p2m_mmio_direct_dev);

Coding style.


+if ( res < 0 )
+{
+dprintk(XENLOG_ERR,
+"Failed to map %"PRIpaddr" to the guest 
at%"PRIpaddr"\n",
+mstart, gstart);

Similarly, this wants to be a printk.


+return -EFAULT;
+}
+}
+
+return 0;
+}
+/*
+ * xen,path specifies the corresponding node in the host DT.
+ * Both interrupt mappings and IOMMU settings are based on it,
+ * as they are done based on the corresponding host DT node.
+ */
+else if ( dt_prop_cmp("xen,path", name) == 0 )
+{
+node = dt_find_node_by_path(prop->data);
+if ( node == NULL )
+{
+dprintk(XENLOG_ERR, "Couldn't find node %s in host_dt!\n",
+(char *)prop->data);

Same here.


+return -EINVAL;
+}


I have to admit that I don't know about dom0less feature enough ...


But, shouldn't we check if the device is behind 

Re: [Xen-devel] [PATCH v4 28/46] xen/sched: add code to sync scheduling of all vcpus of a sched unit

[Dario Faggioli]

On Fri, 2019-09-27 at 09:00 +0200, Juergen Gross wrote:
> When switching sched units synchronize all vcpus of the new unit to
> be
> scheduled at the same time.
> 
> A variable sched_granularity is added which holds the number of vcpus
> per schedule unit.
> 
> As tasklets require to schedule the idle unit it is required to set
> the
> tasklet_work_scheduled parameter of do_schedule() to true if any cpu
> covered by the current schedule() call has any pending tasklet work.
> 
> For joining other vcpus of the schedule unit we need to add a new
> softirq SCHED_SLAVE_SOFTIRQ in order to have a way to initiate a
> context switch without calling the generic schedule() function
> selecting the vcpu to switch to, as we already know which vcpu we
> want to run. This has the other advantage not to loose any other
> concurrent SCHEDULE_SOFTIRQ events.
> 
> Signed-off-by: Juergen Gross 
>
Reviewed-by: Dario Faggioli 

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



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

Re: [Xen-devel] [PATCH v7 0/4] Fix PCI passthrough for HVM with stubdomain

[Wei Liu]

On Fri, Sep 27, 2019 at 04:21:55PM +0200, Jan Beulich wrote:
> > 
> > Marek Marczykowski-Górecki (4):
> >   libxl: do not attach xen-pciback to HVM domain, if stubdomain is in use
> >   libxl: attach PCI device to qemu only after setting pciback/pcifront
> >   libxl: don't try to manipulate json config for stubdomain
> >   xen/x86: Allow stubdom access to irq created for msi.
> 
> I did commit the last one, but I'd prefer if one of you could take
> care of the three libxl ones.
> 

I tried to apply the first three. They don't apply cleanly. That's
perhaps due to Anthony's series which got committed recently.

Marek, do you have a branch?

Wei.

> Thanks, Jan

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

Re: [Xen-devel] [PATCH v7 0/4] Fix PCI passthrough for HVM with stubdomain

[Jan Beulich]

Ian, Wei,

On 25.09.2019 04:41, Marek Marczykowski-Górecki  wrote:
> In this version, I drop PHYSDEVOP_interrupt_control patch, since Jan prefer 
> not
> to mix pciif and hypercalls for serving device model. Enabling MSI by the
> stubdomain remains unsolved here, but other patches are improvement anyway.
> 
> Changes in v2:
>  - new "xen/x86: Allow stubdom access to irq created for msi" patch
>  - applied review comments from v1
> Changes is v3:
>  - apply suggestions by Roger
>  - add PHYSDEVOP_msi_msix_set_enable
> Changes in v4:
>  - implement suggestions by Wei, Roger, Jan
>  - plug new physdevop into XSM
> Changes in v5:
>  - rebase on master
>  - rename to PHYSDEVOP_msi_control
>  - move granting access to IRQ into create_irq
> Changes in v6:
>  - simplify granting IRQ access, record dm domid for cleanup
>  - rename to PHYSDEVOP_interrupt_control
>  - include INTx control in the hypercall
> Changes in v7:
>  - update "xen/x86: Allow stubdom access to irq created for msi"
>  - drop "xen/x86: add PHYSDEVOP_interrupt_control"
>  - drop "tools/libxc: add wrapper for PHYSDEVOP_interrupt_control"
> 
> ---
> Cc: Ian Jackson 
> Cc: Wei Liu 
> Cc: Andrew Cooper 
> Cc: George Dunlap 
> Cc: Jan Beulich 
> Cc: Julien Grall 
> Cc: Konrad Rzeszutek Wilk 
> Cc: Stefano Stabellini 
> Cc: Tim Deegan 
> Cc: Anthony PERARD 
> Cc: "Roger Pau Monné" 
> Cc: Suravee Suthikulpanit 
> Cc: Brian Woods 
> Cc: Kevin Tian 
> Cc: Daniel De Graaf 
> 
> Marek Marczykowski-Górecki (4):
>   libxl: do not attach xen-pciback to HVM domain, if stubdomain is in use
>   libxl: attach PCI device to qemu only after setting pciback/pcifront
>   libxl: don't try to manipulate json config for stubdomain
>   xen/x86: Allow stubdom access to irq created for msi.

I did commit the last one, but I'd prefer if one of you could take
care of the three libxl ones.

Thanks, Jan

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

Re: [Xen-devel] [PATCH RFC for-4.13 04/10] xen/arm: Ensure the SSBD workaround is re-enabled right after exiting a guest

[Volodymyr Babchuk]

Julien Grall writes:

> On 27/09/2019 14:33, Volodymyr Babchuk wrote:
>> Julien Grall writes:
>>> On 27/09/2019 13:39, Volodymyr Babchuk wrote:
 Julien Grall writes:
> On 27/09/2019 12:56, Volodymyr Babchuk wrote:
>> Julien Grall writes:
>>
>>> At the moment, SSBD workaround is re-enabled for Xen after interrupts
>>> are unmasked. This means we may end up to execute some part of the
>>> hypervisor if an interrupt is received before the workaround is
>>> re-enabled.
>>>
>>> As the rest of enter_hypervisor_from_guest() does not require to have
>>> interrupts masked, the function is now split in two parts:
>>>1) enter_hypervisor_from_guest_noirq() called with interrupts
>>>   masked.
>> I'm okay with this approach, but I don't like name for
>> enter_hypervisor_from_guest_noirq(). Right now it is doing exactly one
>> thing - mitigates SSBD. So, maybe more appropriate name will be
>> something like "mitigate_ssbd()" ?
>
> If I wanted to call it mitigate_ssbd() I would have implemented
> completely differently. The reason it is like that is because we may
> need more code to be added here in the future (I have Andrii's series
> in mind). So I would rather avoid a further renaming later on and some
> rework.
 Fair enough

>
> Regarding the name, this is a split of
> enter_hypervisor_from_guest(). Hence, why the first path is the
> same. The noirq merely help the user to know what to expect. This is
> better of yet an __ version. Feel free to suggest a better suffix.
 I'm bad at naming things :)
>>>
>>> Me too ;).
>>>

 I understand that is two halves of one function. But func_name_noirq()
 pattern is widely used for other case: when we have func_name_noirq()
 function and some func_name() that disables interrupts like this:

 void func_name()
 {
   disable_irqs();
   func_name_noirq();
   enable_irqs();
 }

 I like principle of least surprise, so it is better to use some other
 naming pattern there.
>>>
>>> I can't find any function suffixed with _noirq in Xen. So I don't
>>> think this would be a major issue here.
>> Yes, there are no such functions in Xen. But it may confuse developers
>> who come from another projects.
>
> Well, each projects have their own style. So there are always some
> adaptations needed to move to a new project. What matters is the
> documentation clarifies what is the exact use. But...
>
>>

 maybe something like enter_hypervisor_from_guest_pt1() and
 enter_hypervisor_from_guest_pt2()?
>>> Hmmm, it reminds me uni when we had to limit function size to 20 lines :).
>>>
>>> I chose _noirq because the other name I had in mind was quite
>>> verbose. I was thinking:
>>> enter_hypervisor_from_guest_before_interrupts().
>> A was thinking about something like this too.
>> What about enter_hypervisor_from_guest_preirq()?
>
> ... this would be indeed better.

>>
>> I think that "_pre" better shows the relation to
>> enter_hypervisor_from_guest()
>>
>>>

 Or maybe, we should not split the function at all? Instead, we enable
 interrupts right in the middle of it.
>>>
>>> I thought about this but I didn't much like the resulting code.
>>>
>>> The instruction to unmask interrupts requires to take an immediate
>>> (indicates which interrupts to unmask). As not all the traps require
>>> to unmask the same interrupts, we would end up to have to a bunch of
>>> if in the code to select the right unmasking.
>> Ah, yes, this is the problem. We can provide callback to
>> enter_hypervisor_from_guest().
>
> I am not sure what you mean by this. Do you mean a callback that will
> unmask the interrupts?
Yes. You can pass function pointer to enter_hypervisor_from_guest(). To
a function, that will unmask the interrupts. I'm sure that guest_vector
macro can generate it for you. Something like this:

.macro  guest_vector compat, iflags, trap, save_x0_x1=1
entry   hyp=0, compat=\compat, save_x0_x1=\save_x0_x1
/*
 * The vSError will be checked while SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
 * is not set. If a vSError took place, the initial exception will be
 * skipped. Exit ASAP
 */
ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
"nop; nop",
SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
ldr x0, =1f
bl  enter_hypervisor_from_guest
mov x0, sp
bl  do_trap_\trap
b   1f
2:
msr daifclr, \iflags
ret
1:
exithyp=0, compat=\compat
.endm



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

Re: [Xen-devel] [PATCH v2 5/6] Add guide on Communication Best Practice

[Jan Beulich]

On 27.09.2019 12:17, Lars Kurth wrote:
> Can I maybe get you to reconsider and re-review the next version from the
> view point of an author and maybe make suggestions on how to create more
> balance

I'll certainly make an attempt.

Jan

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

Re: [Xen-devel] [RFC PATCH 71/84] x86/setup: start tearing down the direct map.

[Jan Beulich]

On 27.09.2019 16:02, hong...@amazon.com wrote:
> On 27/09/2019 14:00, Jan Beulich wrote:
>> On 27.09.2019 14:54, hong...@amazon.com wrote:
>>
>> Pre-populate? There's some conceptional question then: When the
>> direct map is gone, are you mapping Xen heap pages into the place
>> they'd have lived at in the direct map? I'm not convinced that's
>> what we want. In fact I'm not convinced we'd want to retain the
>> distinction between Xen heap and domain heap then any further -
>> there's no reason anymore at that point (afaict).
> 
> Yes. My patches map xenheap pages to where they would have lived on the 
> direct 
> map region, and unmap when xenheap pages are freed. The original proposal was 
> to use vmap() which we find difficult to implement.
> 
> - vmap takes an array of mfns. Mapping a large region require 
> allocating/freeing memory for a large array of mfns, unless we change or add 
> another vmap variant.
> - va<->pa conversion. Mapping xenheap to direct map region makes all the 
> xenheap conversion macros still work. The vmap proposal needs to add another 
> field in page_info (breaking the power of 2) or to have a separate structure 
> somewhere else for va/pa conversion.

But then why do the initial so many patches (inherited from Wei)
convert from domheap to xenheap allocations at all? If your
approach is to be at least an intermediate goal, then I think the
order of changes should be such that on-demand mapping of xenheap
pages occurs first, and then the xenheap -> domheap conversion
can happen in basically arbitrarily small steps.

Jan

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

Re: [Xen-devel] [PATCH v11 6/7] microcode: rendezvous CPUs in NMI handler and load ucode

[Chao Gao]

On Fri, Sep 27, 2019 at 03:55:00PM +0200, Jan Beulich wrote:
>On 27.09.2019 15:53, Chao Gao wrote:
>> On Fri, Sep 27, 2019 at 12:19:22PM +0200, Jan Beulich wrote:
>>> On 26.09.2019 15:53, Chao Gao wrote:
 @@ -420,14 +498,23 @@ static int control_thread_fn(const struct 
 microcode_patch *patch)
  return ret;
  }
  
 -/* Let primary threads load the given ucode update */
 -set_state(LOADING_ENTER);
 -
 +/* Control thread loads ucode first while others are in NMI handler. 
 */
  ret = microcode_ops->apply_microcode(patch);
  if ( !ret )
  atomic_inc(_updated);
  atomic_inc(_out);
  
 +if ( ret == -EIO )
 +{
 +printk(XENLOG_ERR
 +   "Late loading aborted: CPU%u failed to update ucode\n", 
 cpu);
 +set_state(LOADING_EXIT);
 +return ret;
 +}
 +
 +/* Let primary threads load the given ucode update */
 +set_state(LOADING_ENTER);
>>>
>>> While the description goes to some lengths to explain this ordering of
>>> updates, I still don't really see the point: How is it better for the
>>> control CPU to have updated its ucode early and then hit an NMI before
>>> the other CPUs have even started updating, than the other way around
>>> in the opposite case?
>> 
>> We want to be conservative here. If an ucode is to update something
>> shared by a whole socket, for the latter case, control thread may
>> be accessing things that are being updating by the ucode loading on
>> other cores. It is not safe, just like sibling thread isn't expected
>> to access features exposed by the old ucode when primary thread is
>> loading ucode.
>
>Ah yes, considering a socket-wide effect didn't occur to me (although
>it should have). So if you mention this aspect in the description, I
>think I'm going to be fine with the change in this regard. Yet (as so
>often) this raises another question: What about "secondary" sockets?
>Shouldn't we entertain a similar two-step approach there then?

No. The two-step approach is because control thread cannot call
self_nmi() in case of triggering unknown_nmi_error() and what is done
in the main NMI handler isn't well controlled. All cores on other
sockets will rendezvous in NMI handler. It means every core's behavior
on other sockets is well controlled.

Thanks
Chao

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

Re: [Xen-devel] [PATCH RFC for-4.13 04/10] xen/arm: Ensure the SSBD workaround is re-enabled right after exiting a guest

[Julien Grall]

On 27/09/2019 14:33, Volodymyr Babchuk wrote:

Julien Grall writes:

On 27/09/2019 13:39, Volodymyr Babchuk wrote:

Julien Grall writes:

On 27/09/2019 12:56, Volodymyr Babchuk wrote:

Julien Grall writes:


At the moment, SSBD workaround is re-enabled for Xen after interrupts
are unmasked. This means we may end up to execute some part of the
hypervisor if an interrupt is received before the workaround is
re-enabled.

As the rest of enter_hypervisor_from_guest() does not require to have
interrupts masked, the function is now split in two parts:
   1) enter_hypervisor_from_guest_noirq() called with interrupts
  masked.

I'm okay with this approach, but I don't like name for
enter_hypervisor_from_guest_noirq(). Right now it is doing exactly one
thing - mitigates SSBD. So, maybe more appropriate name will be
something like "mitigate_ssbd()" ?


If I wanted to call it mitigate_ssbd() I would have implemented
completely differently. The reason it is like that is because we may
need more code to be added here in the future (I have Andrii's series
in mind). So I would rather avoid a further renaming later on and some
rework.

Fair enough



Regarding the name, this is a split of
enter_hypervisor_from_guest(). Hence, why the first path is the
same. The noirq merely help the user to know what to expect. This is
better of yet an __ version. Feel free to suggest a better suffix.

I'm bad at naming things :)


Me too ;).



I understand that is two halves of one function. But func_name_noirq()
pattern is widely used for other case: when we have func_name_noirq()
function and some func_name() that disables interrupts like this:

void func_name()
{
  disable_irqs();
  func_name_noirq();
  enable_irqs();
}

I like principle of least surprise, so it is better to use some other
naming pattern there.


I can't find any function suffixed with _noirq in Xen. So I don't
think this would be a major issue here.

Yes, there are no such functions in Xen. But it may confuse developers
who come from another projects.


Well, each projects have their own style. So there are always some adaptations 
needed to move to a new project. What matters is the documentation clarifies 
what is the exact use. But...






maybe something like enter_hypervisor_from_guest_pt1() and
enter_hypervisor_from_guest_pt2()?

Hmmm, it reminds me uni when we had to limit function size to 20 lines :).

I chose _noirq because the other name I had in mind was quite
verbose. I was thinking:
enter_hypervisor_from_guest_before_interrupts().

A was thinking about something like this too.
What about enter_hypervisor_from_guest_preirq()?


... this would be indeed better.



I think that "_pre" better shows the relation to
enter_hypervisor_from_guest()





Or maybe, we should not split the function at all? Instead, we enable
interrupts right in the middle of it.


I thought about this but I didn't much like the resulting code.

The instruction to unmask interrupts requires to take an immediate
(indicates which interrupts to unmask). As not all the traps require
to unmask the same interrupts, we would end up to have to a bunch of
if in the code to select the right unmasking.

Ah, yes, this is the problem. We can provide callback to
enter_hypervisor_from_guest().


I am not sure what you mean by this. Do you mean a callback that will unmask the 
interrupts?




Or switch() instead of multiple ifs. Maybe in some helper function.


Well, my point about "ifs" is that you add a few branch instruction for 
something that can mostly be static (we will always unmask the same interrupts 
for a given exception).


Anyway, such solutions is a no-go for me. This is only muddying the code and I 
care about long-term maintenance.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [RFC PATCH 71/84] x86/setup: start tearing down the direct map.

[hongyax]

On 27/09/2019 14:00, Jan Beulich wrote:

On 27.09.2019 14:54, hong...@amazon.com wrote:

Pre-populate? There's some conceptional question then: When the
direct map is gone, are you mapping Xen heap pages into the place
they'd have lived at in the direct map? I'm not convinced that's
what we want. In fact I'm not convinced we'd want to retain the
distinction between Xen heap and domain heap then any further -
there's no reason anymore at that point (afaict).


Yes. My patches map xenheap pages to where they would have lived on the direct 
map region, and unmap when xenheap pages are freed. The original proposal was 
to use vmap() which we find difficult to implement.


- vmap takes an array of mfns. Mapping a large region require 
allocating/freeing memory for a large array of mfns, unless we change or add 
another vmap variant.
- va<->pa conversion. Mapping xenheap to direct map region makes all the 
xenheap conversion macros still work. The vmap proposal needs to add another 
field in page_info (breaking the power of 2) or to have a separate structure 
somewhere else for va/pa conversion.


Of course, we could change all the code for xenheap to use the same domheap 
mapping structure, which is probably another large patch series.


Hongyan

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

[Xen-devel] [ANNOUNCE] Call for agenda items for Oct, 10th Community Call @ 15:00 UTC - One week later than normal

[Lars Kurth]

Hi all,

the proposed agenda is in 
https://cryptpad.fr/pad/#/2/pad/edit/4FGEw81flPUiivkjkuvQJ-CK/embed/present/and 
you can edit to add items
Alternatively, you can reply to this mail directly

Agenda items appreciated a few days before the call: please put your name 
besides items if you edit the document

Best Regards
Lars
P.S.: If you want to be added or removed from the CC list please reply privately

== Dial-in Information ==
## Meeting time
15:00 - 16:00 UTC
Further International meeting times: 
https://www.timeanddate.com/worldclock/meetingdetails.html?year=2019=10=10=15=0=0=225=224=24=179=136=37=33

## Dial in details
Web: https://www.gotomeet.me/larskurth
You can also dial in using your phone.
Access Code: 906-886-965
China (Toll Free): 4008 811084
Germany: +49 692 5736 7317
Poland (Toll Free): 00 800 1124759
Ukraine (Toll Free): 0 800 50 1733
United Kingdom: +44 330 221 0088
United States: +1 (571) 317-3129
Spain: +34 932 75 2004

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

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


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

Re: [Xen-devel] [PATCH v11 6/7] microcode: rendezvous CPUs in NMI handler and load ucode

[Jan Beulich]

On 27.09.2019 15:53, Chao Gao wrote:
> On Fri, Sep 27, 2019 at 12:19:22PM +0200, Jan Beulich wrote:
>> On 26.09.2019 15:53, Chao Gao wrote:
>>> @@ -420,14 +498,23 @@ static int control_thread_fn(const struct 
>>> microcode_patch *patch)
>>>  return ret;
>>>  }
>>>  
>>> -/* Let primary threads load the given ucode update */
>>> -set_state(LOADING_ENTER);
>>> -
>>> +/* Control thread loads ucode first while others are in NMI handler. */
>>>  ret = microcode_ops->apply_microcode(patch);
>>>  if ( !ret )
>>>  atomic_inc(_updated);
>>>  atomic_inc(_out);
>>>  
>>> +if ( ret == -EIO )
>>> +{
>>> +printk(XENLOG_ERR
>>> +   "Late loading aborted: CPU%u failed to update ucode\n", 
>>> cpu);
>>> +set_state(LOADING_EXIT);
>>> +return ret;
>>> +}
>>> +
>>> +/* Let primary threads load the given ucode update */
>>> +set_state(LOADING_ENTER);
>>
>> While the description goes to some lengths to explain this ordering of
>> updates, I still don't really see the point: How is it better for the
>> control CPU to have updated its ucode early and then hit an NMI before
>> the other CPUs have even started updating, than the other way around
>> in the opposite case?
> 
> We want to be conservative here. If an ucode is to update something
> shared by a whole socket, for the latter case, control thread may
> be accessing things that are being updating by the ucode loading on
> other cores. It is not safe, just like sibling thread isn't expected
> to access features exposed by the old ucode when primary thread is
> loading ucode.

Ah yes, considering a socket-wide effect didn't occur to me (although
it should have). So if you mention this aspect in the description, I
think I'm going to be fine with the change in this regard. Yet (as so
often) this raises another question: What about "secondary" sockets?
Shouldn't we entertain a similar two-step approach there then?

Jan

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

Re: [Xen-devel] [win-pv-devel] [Vote] XCP-ng subproject proposal

[Paul Durrant]

+1 from me

On Fri, 27 Sep 2019 at 14:00, Lars Kurth  wrote:
>
>
>
> On 9 Sep 2019, at 15:44, Lars Kurth  wrote:
>
> Hello everyone,
>
> Olivier had posted an RFC for this proposal on xen-devel@- see 
> https://xen.markmail.org/thread/ermnrb3ps3okvnjr
>
> The proposal also has been discussed by the Advisory Board and was approved
>
> However, for the proposal to fully pass the proposal must be run by past all 
> mature subproject, which are Hypervisors, Windows PV Drivers and XAPI (see 
> https://xenproject.org/developers/governance/#project-decisions). People 
> listed under Project team visible on the right columns of following pages can 
> vote
> * https://xenproject.org/developers/teams/xen-hypervisor/ - already voted: 
> Jan, Ian, Wei, George
> * https://xenproject.org/developers/teams/windows-pv-drivers/
> * https://xenproject.org/developers/teams/xen-api/
>
> The RFC proposal has passed the Hypervisor team with 4/8 votes (see 
> https://xen.markmail.org/thread/ermnrb3ps3okvnjr), but more support would be 
> appreciated
>
> The proposal is attached below. Please vote before next Tuesday
>
> Best Regards
> Lars
>
>
> Hi all.
> so no more votes which means the proposal has passed
> Lard
>
> ___
> win-pv-devel mailing list
> win-pv-de...@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/win-pv-devel

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

Re: [Xen-devel] [PATCH v11 6/7] microcode: rendezvous CPUs in NMI handler and load ucode

[Chao Gao]

On Fri, Sep 27, 2019 at 12:19:22PM +0200, Jan Beulich wrote:
>On 26.09.2019 15:53, Chao Gao wrote:
>> @@ -105,23 +110,42 @@ void __init microcode_set_module(unsigned int idx)
>>  }
>>  
>>  /*
>> - * The format is '[|scan]'. Both options are optional.
>> + * The format is '[|scan, nmi=]'. Both options are optional.
>>   * If the EFI has forced which of the multiboot payloads is to be used,
>> - * no parsing will be attempted.
>> + * only nmi= is parsed.
>>   */
>>  static int __init parse_ucode(const char *s)
>>  {
>> -const char *q = NULL;
>> +const char *ss;
>> +int val, rc = 0;
>>  
>> -if ( ucode_mod_forced ) /* Forced by EFI */
>> -   return 0;
>> +do {
>> +ss = strchr(s, ',');
>> +if ( !ss )
>> +ss = strchr(s, '\0');
>>  
>> -if ( !strncmp(s, "scan", 4) )
>> -ucode_scan = 1;
>> -else
>> -ucode_mod_idx = simple_strtol(s, , 0);
>> +if ( (val = parse_boolean("nmi", s, ss)) >= 0 )
>> +ucode_in_nmi = val;
>> +else if ( !ucode_mod_forced ) /* Not forced by EFI */
>> +{
>> +const char *q = NULL;
>> +
>> +if ( !strncmp(s, "scan", 4) )
>> +{
>> +ucode_scan = true;
>
>I guess it would have resulted in more consistent code if you had
>used parse_boolean() here, too.
>
>> @@ -222,6 +246,8 @@ const struct microcode_ops *microcode_ops;
>>  static DEFINE_SPINLOCK(microcode_mutex);
>>  
>>  DEFINE_PER_CPU(struct cpu_signature, cpu_sig);
>> +/* Store error code of the work done in NMI handler */
>> +DEFINE_PER_CPU(int, loading_err);
>
>static
>
>> @@ -356,42 +383,88 @@ static void set_state(unsigned int state)
>>  smp_wmb();
>>  }
>>  
>> -static int secondary_thread_fn(void)
>> +static int secondary_nmi_work(void)
>>  {
>> -unsigned int primary = cpumask_first(this_cpu(cpu_sibling_mask));
>> +cpumask_set_cpu(smp_processor_id(), _callin_map);
>>  
>> -if ( !wait_for_state(LOADING_CALLIN) )
>> -return -EBUSY;
>> +return wait_for_state(LOADING_EXIT) ? 0 : -EBUSY;
>> +}
>> +
>> +static int primary_thread_work(const struct microcode_patch *patch)
>> +{
>> +int ret;
>>  
>>  cpumask_set_cpu(smp_processor_id(), _callin_map);
>>  
>> -if ( !wait_for_state(LOADING_EXIT) )
>> +if ( !wait_for_state(LOADING_ENTER) )
>>  return -EBUSY;
>>  
>> -/* Copy update revision from the primary thread. */
>> -this_cpu(cpu_sig).rev = per_cpu(cpu_sig, primary).rev;
>> +ret = microcode_ops->apply_microcode(patch);
>> +if ( !ret )
>> +atomic_inc(_updated);
>> +atomic_inc(_out);
>>  
>> -return 0;
>> +return ret;
>>  }
>>  
>> -static int primary_thread_fn(const struct microcode_patch *patch)
>> +static int primary_nmi_work(const struct microcode_patch *patch)
>> +{
>> +return primary_thread_work(patch);
>> +}
>
>Why this wrapper? The function signatures are identical. I guess
>you want to emphasize the environment the function is to be used
>in, so perhaps fine despite the redundancy. At least there's no
>address taken of this function, so the compiler can eliminate it.
>
>> +static int secondary_thread_fn(void)
>> +{
>>  if ( !wait_for_state(LOADING_CALLIN) )
>>  return -EBUSY;
>>  
>> -cpumask_set_cpu(smp_processor_id(), _callin_map);
>> +self_nmi();
>>  
>> -if ( !wait_for_state(LOADING_ENTER) )
>> +/* Copy update revision from the primary thread. */
>> +this_cpu(cpu_sig).rev =
>> +per_cpu(cpu_sig, cpumask_first(this_cpu(cpu_sibling_mask))).rev;
>
>_alternative_instructions() takes specific care to avoid relying on
>the NMI potentially not arriving synchronously (in which case you'd
>potentially copy a not-yet-updated CPU signature above). I think the
>same care wants applying here, which I guess would be another
>
>wait_for_state(LOADING_EXIT);
>
>> +return this_cpu(loading_err);
>> +}
>> +
>> +static int primary_thread_fn(const struct microcode_patch *patch)
>> +{
>> +if ( !wait_for_state(LOADING_CALLIN) )
>>  return -EBUSY;
>>  
>> -ret = microcode_ops->apply_microcode(patch);
>> -if ( !ret )
>> -atomic_inc(_updated);
>> -atomic_inc(_out);
>> +if ( ucode_in_nmi )
>> +{
>> +self_nmi();
>> +return this_cpu(loading_err);
>
>Same here than, to protect against returning a not-yet-updated error
>indicator.
>
>> @@ -420,14 +498,23 @@ static int control_thread_fn(const struct 
>> microcode_patch *patch)
>>  return ret;
>>  }
>>  
>> -/* Let primary threads load the given ucode update */
>> -set_state(LOADING_ENTER);
>> -
>> +/* Control thread loads ucode first while others are in NMI handler. */
>>  ret = microcode_ops->apply_microcode(patch);
>>  if ( !ret )
>>  atomic_inc(_updated);
>>  atomic_inc(_out);
>>  
>> +if ( ret == -EIO )
>> +{
>> +printk(XENLOG_ERR
>> +   "Late loading aborted: CPU%u failed to update 

Re: [Xen-devel] [PATCH v1 1/6] docs: add feature document for Xen hypervisor sysfs-like support

[Ian Jackson]

Jürgen Groß writes ("Re: [PATCH v1 1/6] docs: add feature document for Xen 
hypervisor sysfs-like support"):
> On 27.09.19 12:37, Ian Jackson wrote:
> > I guess that keys will be chosen from some limited safe character
> > set ?  What about values ?  Might we create a key whose value contains
> > binary data ?
> 
> I'd go with "[-A-Za-z0-9@_.:()\[\]#,;]*" for keys

I think this is ASCII printing characters with the exception of
  ! " ` $ % ^ & * = + { } ' ~ < > / \ |

I struggle to find a principled explanation for this particular
exclusion set (apart from /), considering that following are
included:
  - @ _ . : ( ) [ ] # , ;

Could we borrow some existing permitted character set ?  If we are
permitting shell metacharacters why not just permit all printable
ASCII except / ?

> and ASCII for values.

Do you mean "any 7-bit byte", or octet values 32-126 (0x20-0x7e)
inclusive, or something else ?

> > Would it be possible to add a script to xen.git which lists the
> > xenhypfs and checks that all the paths are documented ?  We could add
> > a few calls to that to osstest...
> 
> I'd expect some parts to be described rather generically (as can be seen
> in patch 6 for the runtime parameters). Maybe I should add the entries
> with wildcards there.

That would be nice.

> OTOH adding such a script can easily be done later, maybe with some
> tweaks to the path documentation.

Sure.  Having at least some unit tests ought to be on the roadmap for
supported status, but it doesn't need to happen now.

Ian.

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

Re: [Xen-devel] [PATCH RFC for-4.13 04/10] xen/arm: Ensure the SSBD workaround is re-enabled right after exiting a guest

[Volodymyr Babchuk]

Hi,

Julien Grall writes:

> Hi,
>
> On 27/09/2019 13:39, Volodymyr Babchuk wrote:
>> Julien Grall writes:
>>> On 27/09/2019 12:56, Volodymyr Babchuk wrote:
 Julien Grall writes:

> At the moment, SSBD workaround is re-enabled for Xen after interrupts
> are unmasked. This means we may end up to execute some part of the
> hypervisor if an interrupt is received before the workaround is
> re-enabled.
>
> As the rest of enter_hypervisor_from_guest() does not require to have
> interrupts masked, the function is now split in two parts:
>   1) enter_hypervisor_from_guest_noirq() called with interrupts
>  masked.
 I'm okay with this approach, but I don't like name for
 enter_hypervisor_from_guest_noirq(). Right now it is doing exactly one
 thing - mitigates SSBD. So, maybe more appropriate name will be
 something like "mitigate_ssbd()" ?
>>>
>>> If I wanted to call it mitigate_ssbd() I would have implemented
>>> completely differently. The reason it is like that is because we may
>>> need more code to be added here in the future (I have Andrii's series
>>> in mind). So I would rather avoid a further renaming later on and some
>>> rework.
>> Fair enough
>>
>>>
>>> Regarding the name, this is a split of
>>> enter_hypervisor_from_guest(). Hence, why the first path is the
>>> same. The noirq merely help the user to know what to expect. This is
>>> better of yet an __ version. Feel free to suggest a better suffix.
>> I'm bad at naming things :)
>
> Me too ;).
>
>>
>> I understand that is two halves of one function. But func_name_noirq()
>> pattern is widely used for other case: when we have func_name_noirq()
>> function and some func_name() that disables interrupts like this:
>>
>> void func_name()
>> {
>>  disable_irqs();
>>  func_name_noirq();
>>  enable_irqs();
>> }
>>
>> I like principle of least surprise, so it is better to use some other
>> naming pattern there.
>
> I can't find any function suffixed with _noirq in Xen. So I don't
> think this would be a major issue here.
Yes, there are no such functions in Xen. But it may confuse developers
who come from another projects.

>>
>> maybe something like enter_hypervisor_from_guest_pt1() and
>> enter_hypervisor_from_guest_pt2()?
> Hmmm, it reminds me uni when we had to limit function size to 20 lines :).
>
> I chose _noirq because the other name I had in mind was quite
> verbose. I was thinking:
> enter_hypervisor_from_guest_before_interrupts().
A was thinking about something like this too.
What about enter_hypervisor_from_guest_preirq()?

I think that "_pre" better shows the relation to
enter_hypervisor_from_guest()

>
>>
>> Or maybe, we should not split the function at all? Instead, we enable
>> interrupts right in the middle of it.
>
> I thought about this but I didn't much like the resulting code.
>
> The instruction to unmask interrupts requires to take an immediate
> (indicates which interrupts to unmask). As not all the traps require
> to unmask the same interrupts, we would end up to have to a bunch of
> if in the code to select the right unmasking.
Ah, yes, this is the problem. We can provide callback to
enter_hypervisor_from_guest().

Or switch() instead of multiple ifs. Maybe in some helper function.

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

Re: [Xen-devel] [PATCH RFC for-4.13 04/10] xen/arm: Ensure the SSBD workaround is re-enabled right after exiting a guest

[Julien Grall]

Hi,

On 27/09/2019 13:39, Volodymyr Babchuk wrote:

Julien Grall writes:

On 27/09/2019 12:56, Volodymyr Babchuk wrote:

Julien Grall writes:


At the moment, SSBD workaround is re-enabled for Xen after interrupts
are unmasked. This means we may end up to execute some part of the
hypervisor if an interrupt is received before the workaround is
re-enabled.

As the rest of enter_hypervisor_from_guest() does not require to have
interrupts masked, the function is now split in two parts:
  1) enter_hypervisor_from_guest_noirq() called with interrupts
 masked.

I'm okay with this approach, but I don't like name for
enter_hypervisor_from_guest_noirq(). Right now it is doing exactly one
thing - mitigates SSBD. So, maybe more appropriate name will be
something like "mitigate_ssbd()" ?


If I wanted to call it mitigate_ssbd() I would have implemented
completely differently. The reason it is like that is because we may
need more code to be added here in the future (I have Andrii's series
in mind). So I would rather avoid a further renaming later on and some
rework.

Fair enough



Regarding the name, this is a split of
enter_hypervisor_from_guest(). Hence, why the first path is the
same. The noirq merely help the user to know what to expect. This is
better of yet an __ version. Feel free to suggest a better suffix.

I'm bad at naming things :)


Me too ;).



I understand that is two halves of one function. But func_name_noirq()
pattern is widely used for other case: when we have func_name_noirq()
function and some func_name() that disables interrupts like this:

void func_name()
{
 disable_irqs();
 func_name_noirq();
 enable_irqs();
}

I like principle of least surprise, so it is better to use some other
naming pattern there.


I can't find any function suffixed with _noirq in Xen. So I don't think this 
would be a major issue here.




maybe something like enter_hypervisor_from_guest_pt1() and
enter_hypervisor_from_guest_pt2()?

Hmmm, it reminds me uni when we had to limit function size to 20 lines :).

I chose _noirq because the other name I had in mind was quite verbose. I was 
thinking: enter_hypervisor_from_guest_before_interrupts().




Or maybe, we should not split the function at all? Instead, we enable
interrupts right in the middle of it.


I thought about this but I didn't much like the resulting code.

The instruction to unmask interrupts requires to take an immediate (indicates 
which interrupts to unmask). As not all the traps require to unmask the same 
interrupts, we would end up to have to a bunch of if in the code to select the 
right unmasking.


So the split solution was the best I had in mind. I am open to better suggestion 
here.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [RFC PATCH 58/84] x86/mm: fix leaks in map_xen_pagetable.

[hongyax]

On 26/09/2019 14:16, Wei Liu wrote:

On Thu, Sep 26, 2019 at 10:46:21AM +0100, hong...@amazon.com wrote:


diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index b2b2edbed1..145c5ab47c 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5160,6 +5160,7 @@ int map_pages_to_xen(
   !(l2e_get_flags(ol2e) & _PAGE_PSE) )
  free_xen_pagetable(l2e_get_mfn(ol2e));
  }
+UNMAP_XEN_PAGETABLE(l2t);


This is presumably the issue you try to fix.



Yes. Actually this patch fixes two issues, this is the first one.


diff --git a/xen/common/vmap.c b/xen/common/vmap.c
index faebc1ddf1..fcdb8495c8 100644
--- a/xen/common/vmap.c
+++ b/xen/common/vmap.c


I fail to see why you need to change vmap to fix a leak somewhere else.



The second leak is that after the patches, we cannot just call vmap_to_mfn() 
because it calls virt_to_xen_l1e() under the hood which maps a page. We have to 
unmap it, therefore I modified the vmap_to_mfn to also do the unmapping. This 
is a separate issue than the first one, so maybe I could split the patch into two.



I guess I will need to wait for your branch to have a closer look.

Wei.




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

Re: [Xen-devel] [RFC PATCH 71/84] x86/setup: start tearing down the direct map.

[Julien Grall]

Hi,

On 27/09/2019 13:54, hong...@amazon.com wrote:

On 26/09/2019 15:26, Wei Liu wrote:

On Thu, Sep 26, 2019 at 10:46:34AM +0100, hong...@amazon.com wrote:

From: Hongyan Xia 

Signed-off-by: Hongyan Xia 
---
  xen/arch/x86/setup.c    | 4 ++--
  xen/common/page_alloc.c | 2 +-
  2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index e964c032f6..3dc2fad987 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1367,7 +1367,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
  if ( map_e < end )
  {
-    map_pages_to_xen((unsigned long)__va(map_e), 
maddr_to_mfn(map_e),

+    map_pages_to_xen((unsigned long)__va(map_e), INVALID_MFN,
   PFN_DOWN(end - map_e), PAGE_HYPERVISOR);


Why don't you just remove the calls to map_pages_to_xen?



My intention is to pre-populate the range so that we don't have to do so later 
when there are xenheap allocations. But of course if there is superpage merging 
or shattering, page tables will be removed or allocated anyway. I will remove 
the calls in the next revision.


How about using populate_pt_range() in that case? This will pre-populate the 
page-tables for mapping with small pages.


I haven't fully read the series yet. But I would assume that only memory 
allocated for Xen internal would be kept mapped. Guest memory would still be 
unmapped, right?


If so, I don't think we often do big allocation for Xen. So it is probably more 
likely to use small pages. In that case, it would be fine to pre-allocate pages.


In another hand, Xen doesn't use a lot of memory (if you compare to guest 
memory). So maybe pre-populating the page-tables would be a waste of memory.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [RFC PATCH 71/84] x86/setup: start tearing down the direct map.

[Jan Beulich]

On 27.09.2019 14:54, hong...@amazon.com wrote:
> On 26/09/2019 15:26, Wei Liu wrote:
>> On Thu, Sep 26, 2019 at 10:46:34AM +0100, hong...@amazon.com wrote:
>>> --- a/xen/arch/x86/setup.c
>>> +++ b/xen/arch/x86/setup.c
>>> @@ -1367,7 +1367,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>>>   
>>>   if ( map_e < end )
>>>   {
>>> -map_pages_to_xen((unsigned long)__va(map_e), 
>>> maddr_to_mfn(map_e),
>>> +map_pages_to_xen((unsigned long)__va(map_e), INVALID_MFN,
>>>PFN_DOWN(end - map_e), PAGE_HYPERVISOR);
>>
>> Why don't you just remove the calls to map_pages_to_xen?
>>
> 
> My intention is to pre-populate the range so that we don't have to do so 
> later 
> when there are xenheap allocations. But of course if there is superpage 
> merging 
> or shattering, page tables will be removed or allocated anyway. I will remove 
> the calls in the next revision.

Pre-populate? There's some conceptional question then: When the
direct map is gone, are you mapping Xen heap pages into the place
they'd have lived at in the direct map? I'm not convinced that's
what we want. In fact I'm not convinced we'd want to retain the
distinction between Xen heap and domain heap then any further -
there's no reason anymore at that point (afaict).

Jan

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

Re: [Xen-devel] [Vote] XCP-ng subproject proposal

[Lars Kurth]

> On 9 Sep 2019, at 15:44, Lars Kurth  wrote:
> 
> Hello everyone,
> 
> Olivier had posted an RFC for this proposal on xen-devel@- see 
> https://xen.markmail.org/thread/ermnrb3ps3okvnjr 
>  
> 
> The proposal also has been discussed by the Advisory Board and was approved
> 
> However, for the proposal to fully pass the proposal must be run by past all 
> mature subproject, which are Hypervisors, Windows PV Drivers and XAPI (see 
> https://xenproject.org/developers/governance/#project-decisions 
> ). People 
> listed under Project team visible on the right columns of following pages can 
> vote
> * https://xenproject.org/developers/teams/xen-hypervisor/ 
>  - already voted: 
> Jan, Ian, Wei, George
> * https://xenproject.org/developers/teams/windows-pv-drivers/ 
> 
> * https://xenproject.org/developers/teams/xen-api/ 
> 
> 
> The RFC proposal has passed the Hypervisor team with 4/8 votes (see 
> https://xen.markmail.org/thread/ermnrb3ps3okvnjr 
> ), but more support would 
> be appreciated
> 
> The proposal is attached below. Please vote before next Tuesday
> 
> Best Regards
> Lars

Hi all.
so no more votes which means the proposal has passed
Lard

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

Re: [Xen-devel] [PATCH for-next RFC 4/8] x86: factor out xen variants for hypervisor setup code

[Jan Beulich]

On 27.09.2019 14:47, Wei Liu wrote:
> On Fri, Sep 27, 2019 at 01:39:14PM +0200, Jan Beulich wrote:
>> On 27.09.2019 13:30, Wei Liu wrote:
>>> On Wed, Sep 25, 2019 at 12:23:54PM +0200, Roger Pau Monné wrote:
 Also it might be nice to introduce something like:

 enum guest_type {
 XEN_GUEST,
 } guest_type;

>>>
>>> This works for me.
>>>
 So that you can add a switch here, even if the only case is Xen ATM. I
 guess this will be done in a later patch, or introduce an
 hypervisor_ops struct that contain pointers to functions to allow for
 different implementations.

>>>
>>> I debated this. These changes require more code churn with no apparent
>>> benefit, but if people agree this is better I can make those changes.
>>
>> Well, if the expectation is for the enum to grow (even just by one
>> further entry), then yes, I'd agree that a switch() would be nice.
> 
> Not sure if you notice comments in a later email.
> 
> Do you prefer enum+switch solution or hypervisor_op struct?

Hard to tell without knowing how many switch() there would
end up being. The more of them, the better I'd like the ops
structure variant.

Jan

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

Re: [Xen-devel] [RFC PATCH 71/84] x86/setup: start tearing down the direct map.

[hongyax]

On 26/09/2019 15:26, Wei Liu wrote:

On Thu, Sep 26, 2019 at 10:46:34AM +0100, hong...@amazon.com wrote:

From: Hongyan Xia 

Signed-off-by: Hongyan Xia 
---
  xen/arch/x86/setup.c| 4 ++--
  xen/common/page_alloc.c | 2 +-
  2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index e964c032f6..3dc2fad987 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1367,7 +1367,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
  
  if ( map_e < end )

  {
-map_pages_to_xen((unsigned long)__va(map_e), 
maddr_to_mfn(map_e),
+map_pages_to_xen((unsigned long)__va(map_e), INVALID_MFN,
   PFN_DOWN(end - map_e), PAGE_HYPERVISOR);


Why don't you just remove the calls to map_pages_to_xen?



My intention is to pre-populate the range so that we don't have to do so later 
when there are xenheap allocations. But of course if there is superpage merging 
or shattering, page tables will be removed or allocated anyway. I will remove 
the calls in the next revision.



  init_boot_pages(map_e, end);
  map_e = end;
@@ -1382,7 +1382,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
  }
  if ( s < map_s )
  {
-map_pages_to_xen((unsigned long)__va(s), maddr_to_mfn(s),
+map_pages_to_xen((unsigned long)__va(s), INVALID_MFN,
   PFN_DOWN(map_s - s), PAGE_HYPERVISOR);
  init_boot_pages(s, map_s);
  }
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index a00db4c0d9..deeeac065c 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -2157,7 +2157,7 @@ void *alloc_xenheap_pages(unsigned int order, unsigned 
int memflags)
  map_pages_to_xen((unsigned long)ret, page_to_mfn(pg),
   1UL << order, PAGE_HYPERVISOR);
  
-return page_to_virt(pg);

+return ret;


This hunk is a fix to a previous patch. It doesn't below here.


Noted.

Hongyan

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

Re: [Xen-devel] [PATCH] iommu: fix PVH dom0 settings

[Jan Beulich]

On 27.09.2019 14:28, Roger Pau Monné  wrote:
> On Fri, Sep 27, 2019 at 12:45:54PM +0100, Paul Durrant wrote:
>> --- a/xen/drivers/passthrough/iommu.c
>> +++ b/xen/drivers/passthrough/iommu.c
>> @@ -154,13 +154,13 @@ custom_param("dom0-iommu", parse_dom0_iommu_param);
>>  
>>  static void __hwdom_init check_hwdom_reqs(struct domain *d)
>>  {
>> -if ( iommu_hwdom_none || !paging_mode_translate(d) )
>> +if ( iommu_hwdom_none || !hap_enabled(d) )
> 
> Since dom0 PVH can also be used with shadow paging (not sure how
> useful that is TBH), I'm not sure explicitly checking for hap is fine.
> What about using is_hvm_domain instead?
> 
> That ought to cover both shadow and hap, and a classic PV dom0 won't
> be affected by it (which is the intention).

Oh, indeed - we shouldn't prevent shadow mode use. That'll need to
be an incremental change though, as I've already committed the one
here.

Jan

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

Re: [Xen-devel] [PATCH V5] xen/arm: Restrict "p2m_ipa_bits" according to the IOMMU requirements

[Oleksandr]

On 27.09.19 15:38, Julien Grall wrote:

On 27/09/2019 13:35, Julien Grall wrote:

Hi,


Hi Julien,




On 27/09/2019 12:57, Oleksandr Tyshchenko wrote:

From: Oleksandr Tyshchenko 

There is a strict requirement for the IOMMU which wants to share
the P2M table with the CPU. The IOMMU's Stage-2 input size must be 
equal

to the P2M IPA size. It is not a problem when the IOMMU can support
all values the CPU supports. In that case, the IOMMU driver would just
use any "p2m_ipa_bits" value as is. But, there are cases when not.

In order to make P2M sharing possible on the platforms which
IOMMUs have a limitation in maximum Stage-2 input size introduce
the following logic.

First initialize the IOMMU subsystem and gather requirements regarding
the maximum IPA bits supported by each IOMMU device to figure out
the minimum value among them. In the P2M code, take into the account
the IOMMU requirements and choose suitable "pa_range" according
to the restricted "p2m_ipa_bits".

Signed-off-by: Oleksandr Tyshchenko 
CC: Julien Grall 


Reviewed-by: Julien Grall 


Committed now. Thank you!


Thank you!


I would like to remind regarding the last patch for this moment:

SUPPORT.md: Describe Renesas IPMMU-VMSA support (Arm)

https://lists.xenproject.org/archives/html/xen-devel/2019-09/msg02768.html


Thank you in advance.


--
Regards,

Oleksandr Tyshchenko


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

Re: [Xen-devel] [PATCH v4 24/46] xen: switch from for_each_vcpu() to for_each_sched_unit()

[Jan Beulich]

On 27.09.2019 11:32, Dario Faggioli wrote:
> On Fri, 2019-09-27 at 09:00 +0200, Juergen Gross wrote:
>> Where appropriate switch from for_each_vcpu() to
>> for_each_sched_unit()
>> in order to prepare core scheduling.
>>
>> As it is beneficial once here and for sure in future add a
>> unit_scheduler() helper and let vcpu_scheduler() use it.
>>
>> Signed-off-by: Juergen Gross 
>>
> Reviewed-by: Dario Faggioli 

And the small non-scheduler change here
Acked-by: Jan Beulich 

Jan

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

Re: [Xen-devel] [PATCH RFC for-4.13 03/10] xen/arm: traps: Rework entry/exit from the guest path

[Volodymyr Babchuk]

Julien Grall writes:

> Hi,
>
> On 27/09/2019 13:27, Volodymyr Babchuk wrote:
>>
>> Julien Grall writes:
>>
>>> On 27/09/2019 12:45, Volodymyr Babchuk wrote:

 Julien,
>>>
>>> Hi...
>>>
 Julien Grall writes:

> At the moment, enter_hypervisor_head() and leave_hypervisor_tail() are
> used to deal with actions to be done before/after any guest request is
> handled.
>
> While they are meant to work in pair, the former is called for most of
> the traps, including traps from the same exception level (i.e.
> hypervisor) whilst the latter will only be called when returning to the
> guest.
>
> As pointed out, the enter_hypervisor_head() is not called from all the
> traps, so this makes potentially difficult to extend it for the dealing
> with same exception level.
>
> Furthermore, some assembly only path will require to call
> enter_hypervisor_tail(). So the function is now directly call by
> assembly in for guest vector only. This means that the check whether we
> are called in a guest trap can now be removed.
>
> Take the opportunity to rename enter_hypervisor_tail() and
> leave_hypervisor_tail() to something more meaningful and document them.
> This should help everyone to understand the purpose of the two
> functions.
>
> Signed-off-by: Julien Grall 
>
> ---
>
> I haven't done the 32-bits part yet. I wanted to gather feedback before
> looking in details how to integrate that with Arm32.
 I'm looking at patches one by one and it is looking okay so far.


> ---
>xen/arch/arm/arm64/entry.S |  4 ++-
>xen/arch/arm/traps.c   | 71 
> ++
>2 files changed, 37 insertions(+), 38 deletions(-)
>
> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
> index 40d9f3ec8c..9eafae516b 100644
> --- a/xen/arch/arm/arm64/entry.S
> +++ b/xen/arch/arm/arm64/entry.S
> @@ -147,7 +147,7 @@
>
>.if \hyp == 0 /* Guest mode */
>
> -bl  leave_hypervisor_tail /* Disables interrupts on return */
> +bl  leave_hypervisor_to_guest /* Disables interrupts on 
> return */
>
>exit_guest \compat
>
> @@ -175,6 +175,8 @@
>SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
>msr daifclr, \iflags
>mov x0, sp
 Looks like this mov can be removed (see commend below).

> +bl  enter_hypervisor_from_guest
> +mov x0, sp
>bl  do_trap_\trap
>1:
>exithyp=0, compat=\compat
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index a3b961bd06..20ba34ec91 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2006,47 +2006,46 @@ static inline bool needs_ssbd_flip(struct vcpu *v)
> cpu_require_ssbd_mitigation();
>}
>
> -static void enter_hypervisor_head(struct cpu_user_regs *regs)
> +/*
> + * Actions that needs to be done after exiting the guest and before any
> + * request from it is handled.
 Maybe it is me only, but the phrasing is confusing. I had to read it two
 times before I get it. What about "Actions that needs to be done when
 raising exception level"? Or maybe "Actions that needs to be done when
 switching from guest to hypervisor mode" ?
>>>
>>> Is it a suggestion to replace the full sentence or just the first
>>> before (i.e. before 'and')?
>> This is a suggestion for the first part.
>
> How about:
>
> "Actions that needs to be done after entering the hypervisor from the
> guest and before we handle any request."
Sound perfect.

[...]

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

Re: [Xen-devel] [PATCH v4 20/46] xen: let vcpu_create() select processor

[Jan Beulich]

On 27.09.2019 09:00, Juergen Gross wrote:
> Today there are two distinct scenarios for vcpu_create(): either for
> creation of idle-domain vcpus (vcpuid == processor) or for creation of
> "normal" domain vcpus (including dom0), where the caller selects the
> initial processor on a round-robin scheme of the allowed processors
> (allowed being based on cpupool and affinities).
> 
> Instead of passing the initial processor to vcpu_create() and passing
> on to sched_init_vcpu() let sched_init_vcpu() do the processor
> selection. For supporting dom0 vcpu creation use the node_affinity of
> the domain as a base for selecting the processors. User domains will
> have initially all nodes set, so this is no different behavior compared
> to today. In theory this is not guaranteed as vcpus are created only
> with XEN_DOMCTL_max_vcpus being called, but this call is going to be
> removed in future and the toolstack doesn't call
> XEN_DOMCTL_setnodeaffinity before calling XEN_DOMCTL_max_vcpus.
> 
> To be able to use const struct domain * make cpupool_domain_cpumask()
> take a const domain pointer, too.
> 
> A further simplification is possible by having a single function for
> creating the dom0 vcpus with vcpu_id > 0 and doing the required pinning
> for all vcpus after that. This allows to make sched_set_affinity()
> private to schedule.c and switch it to sched_units easily. Note that
> this functionality is x86 only.
> 
> Signed-off-by: Juergen Gross 
> Acked-by: Julien Grall 

x86 parts:
Reviewed-by: Jan Beulich 
with a couple of nits though (I'll see about taking care of them
while committing):

> @@ -1940,7 +1940,7 @@ static void __init find_gnttab_region(struct domain *d,
>  
>  static int __init construct_domain(struct domain *d, struct kernel_info 
> *kinfo)
>  {
> -int i, cpu;
> +int i;

This would better have become unsigned int, or else ...

> @@ -2003,12 +2003,11 @@ static int __init construct_domain(struct domain *d, 
> struct kernel_info *kinfo)
>  }
>  #endif
>  
> -for ( i = 1, cpu = 0; i < d->max_vcpus; i++ )
> +for ( i = 1; i < d->max_vcpus; i++ )
>  {
> -cpu = cpumask_cycle(cpu, _online_map);
> -if ( vcpu_create(d, i, cpu) == NULL )
> +if ( vcpu_create(d, i) == NULL )
>  {
> -printk("Failed to allocate dom0 vcpu %d on pcpu %d\n", i, cpu);
> +printk("Failed to allocate d0v%u\n", i);

... you should use %d here.

> --- a/xen/arch/x86/dom0_build.c
> +++ b/xen/arch/x86/dom0_build.c
> @@ -165,7 +165,7 @@ custom_param("dom0_max_vcpus", parse_dom0_max_vcpus);
>  static __initdata unsigned int dom0_nr_pxms;
>  static __initdata unsigned int dom0_pxms[MAX_NUMNODES] =
>  { [0 ... MAX_NUMNODES - 1] = ~0 };
> -static __initdata bool dom0_affinity_relaxed;
> +__initdata bool dom0_affinity_relaxed;

The canonical ordering is type then attributes. It's of course not
overly nice to make this and ...

> @@ -196,32 +196,7 @@ static int __init parse_dom0_nodes(const char *s)
>  }
>  custom_param("dom0_nodes", parse_dom0_nodes);
>  
> -static cpumask_t __initdata dom0_cpus;
> -
> -struct vcpu *__init dom0_setup_vcpu(struct domain *d,
> -unsigned int vcpu_id,
> -unsigned int prev_cpu)
> -{
> -unsigned int cpu = cpumask_cycle(prev_cpu, _cpus);
> -struct vcpu *v = vcpu_create(d, vcpu_id, cpu);
> -
> -if ( v )
> -{
> -if ( pv_shim )
> -{
> -sched_set_affinity(v, cpumask_of(vcpu_id), cpumask_of(vcpu_id));
> -}
> -else
> -{
> -if ( !opt_dom0_vcpus_pin && !dom0_affinity_relaxed )
> -sched_set_affinity(v, _cpus, NULL);
> -sched_set_affinity(v, NULL, _cpus);
> -}
> -}
> -
> -return v;
> -}
> -
> +cpumask_t __initdata dom0_cpus;

... this piece of init data non-static, but so be it.

> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -38,6 +38,10 @@
>  #include 
>  #include 
>  
> +#ifdef CONFIG_X86
> +#include 
> +#endif

CONFIG_XEN_GUEST would seem more arch-neutral here.

> @@ -2145,6 +2188,35 @@ void wait(void)
>  schedule();
>  }
>  
> +#ifdef CONFIG_X86
> +void __init sched_setup_dom0_vcpus(struct domain *d)
> +{
> +unsigned int i;
> +struct sched_unit *unit;
> +
> +for ( i = 1; i < d->max_vcpus; i++ )
> +vcpu_create(d, i);
> +
> +for_each_sched_unit ( d, unit )
> +{
> +unsigned int id = unit->unit_id;
> +
> +if ( pv_shim )
> +{
> +sched_set_affinity(unit, cpumask_of(id), cpumask_of(id));
> +}

Unnecessary pair of braces, which is in particular inconsistent with ...

> +else
> +{
> +if ( !opt_dom0_vcpus_pin && !dom0_affinity_relaxed )
> +sched_set_affinity(unit, _cpus, NULL);

... this.

Jan

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

Re: [Xen-devel] [PATCH for-next RFC 4/8] x86: factor out xen variants for hypervisor setup code

[Wei Liu]

On Fri, Sep 27, 2019 at 01:39:14PM +0200, Jan Beulich wrote:
> On 27.09.2019 13:30, Wei Liu wrote:
> > On Wed, Sep 25, 2019 at 12:23:54PM +0200, Roger Pau Monné wrote:
> >> Also it might be nice to introduce something like:
> >>
> >> enum guest_type {
> >> XEN_GUEST,
> >> } guest_type;
> >>
> > 
> > This works for me.
> > 
> >> So that you can add a switch here, even if the only case is Xen ATM. I
> >> guess this will be done in a later patch, or introduce an
> >> hypervisor_ops struct that contain pointers to functions to allow for
> >> different implementations.
> >>
> > 
> > I debated this. These changes require more code churn with no apparent
> > benefit, but if people agree this is better I can make those changes.
> 
> Well, if the expectation is for the enum to grow (even just by one
> further entry), then yes, I'd agree that a switch() would be nice.

Not sure if you notice comments in a later email.

Do you prefer enum+switch solution or hypervisor_op struct?

Wei.

> 
> Jan

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

Re: [Xen-devel] [PATCH RFC for-4.13 09/10] xen/arm: asm: Replace use of ALTERNATIVE with alternative_if

[Volodymyr Babchuk]

Julien Grall writes:

> Hi,
>
> On 27/09/2019 13:11, Volodymyr Babchuk wrote:
>>
>>
>> Julien Grall writes:
>>
>>> Using alternative_if makes the code a bit more streamlined.
>>>
>>> Take the opportunity to use the new auto-nop infrastructure to avoid
>>> counting the number of nop in the else part for arch/arm/arm64/entry.S
>>>
>>> Signed-off-by: Julien Grall 
>>>
>>> ---
>>>  This is pretty much a matter of taste, but at least for arm64 this
>>>  allows us to use the auto-nop infrastructure. So the arm32 is more
>>>  to keep inline with arm64.
>>> ---
>>>   xen/arch/arm/arm32/entry.S | 9 ++---
>>>   xen/arch/arm/arm64/entry.S | 8 +---
>>>   2 files changed, 11 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S
>>> index 0b4cd19abd..1428cd3583 100644
>>> --- a/xen/arch/arm/arm32/entry.S
>>> +++ b/xen/arch/arm/arm32/entry.S
>>> @@ -65,9 +65,12 @@ save_guest_regs:
>>>* If the SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT has been set in the 
>>> cpu
>>>* feature, the checking of pending SErrors will be skipped.
>>>*/
>>> -ALTERNATIVE("nop",
>>> -"b skip_check",
>>> -SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
>>> +alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
>>> +nop
>>> +alternative_else
>>> +b   skip_check
>>> +alternative_endif
>>> +
>> for the arm32 code you can have my r-b:
>> Reviewed-By: Volodymyr Babchuk 
>>
>>>   /*
>>>* Start to check pending virtual abort in the gap of Guest -> HYP
>>>* world switch.
>>> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
>>> index 458d12f188..91cf6ee6f4 100644
>>> --- a/xen/arch/arm/arm64/entry.S
>>> +++ b/xen/arch/arm/arm64/entry.S
>>> @@ -170,9 +170,11 @@
>>>* is not set. If a vSError took place, the initial exception 
>>> will be
>>>* skipped. Exit ASAP
>>>*/
>>> -ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
>>> -"nop; nop",
>>> -SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
>>> +alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
>>> +bl  check_pending_vserror
>>> +cbnzx0, 1f
>>> +alternative_else_nop_endif
>>> +
>> You asked other people to do not introduce new code in one patch and
>> rewrite it in the following patch. But there you are doing exactly the
>> same.
>
> This is a fairly borderline comment knowing that I usually don't
> request clean-up and code consolidation in the same patch.
I understand this. Also I understand why are you asking for clean-up.
No one likes to review the same code twice.

Anyways, I not wanted to be offensive. Sorry for that.


>> I believe, it is possible to move all "alternative" patches to the
>> very beginning of the patch series and only then introduce macro
>> guest_vector.
>
> For a first, the first patch is definitely not new code. This is code
> consolidation and therefore I don't tend to mix the two for
> clarity. So this should have been a patch before the first patch.
>
> Secondly, the first 4 patches are candidate for backport. The rest of
> the series would be good to backport but I am not aware of a critical
> issue in previous Xen release to strongly push for it.
I see. Yes, I'm always forgetting about backporting :(
So, for the rest of the patch:

Reviewed-by: Volodymyr Babchuk 

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

Re: [Xen-devel] [PATCH for-next RFC 4/8] x86: factor out xen variants for hypervisor setup code

[Wei Liu]

On Fri, Sep 27, 2019 at 01:41:59PM +0200, Roger Pau Monné wrote:
> > > 
> > > I wonder, do you also require to map hypervisor data into the guest
> > > physmap when running on HyperV?
> > > 
> > 
> > Yes. There are a lot of comparable concepts in Hyper-V. For example,
> > there is a page called VP assist page which is more or less the same as
> > Xen's vcpuinfo. Its format, content and interfaces may be different, but
> > conceptually it is the same as vcpuinfo.
> > 
> > > Is there a way when running on HyperV to request unused physical
> > > address space ranges? What Xen currently does in init_memmap is quite
> > > crappy.
> > > 
> > 
> > Xen itself still needs to manage the address space, no?
> >
> > I agree the rangeset code in xen.c isn't pretty, but it does the job and
> > is not too intrusive.
> 
> The problem with the current approach is that the nested Xen has no
> way of knowing whether those holes are actually unused, or are MMIO
> regions used by devices for example.
> 
> Hence I wondered if HyperV had a way to signal to guests that a
> physical address range is safe to be used as scratch mapping space. We
> had spoken avoid introducing something in Xen to be able to report to
> guests safe ranges in the physmap to map stuff.

AFAICT Hyper-V TLFS doesn't describe such functionality.

On the other hand, Hyper-V may not need this infrastructure at all
because it doesn't have grant table frame or shared info page. The most
likely outcome is in the next version the memmap stuff will be left to
Xen only until I find a use case for it.

Wei.

> 
> Thanks, Roger.

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

Re: [Xen-devel] [PATCH RFC for-4.13 03/10] xen/arm: traps: Rework entry/exit from the guest path

[Julien Grall]

Hi,

On 27/09/2019 13:27, Volodymyr Babchuk wrote:


Julien Grall writes:


On 27/09/2019 12:45, Volodymyr Babchuk wrote:


Julien,


Hi...


Julien Grall writes:


At the moment, enter_hypervisor_head() and leave_hypervisor_tail() are
used to deal with actions to be done before/after any guest request is
handled.

While they are meant to work in pair, the former is called for most of
the traps, including traps from the same exception level (i.e.
hypervisor) whilst the latter will only be called when returning to the
guest.

As pointed out, the enter_hypervisor_head() is not called from all the
traps, so this makes potentially difficult to extend it for the dealing
with same exception level.

Furthermore, some assembly only path will require to call
enter_hypervisor_tail(). So the function is now directly call by
assembly in for guest vector only. This means that the check whether we
are called in a guest trap can now be removed.

Take the opportunity to rename enter_hypervisor_tail() and
leave_hypervisor_tail() to something more meaningful and document them.
This should help everyone to understand the purpose of the two
functions.

Signed-off-by: Julien Grall 

---

I haven't done the 32-bits part yet. I wanted to gather feedback before
looking in details how to integrate that with Arm32.

I'm looking at patches one by one and it is looking okay so far.



---
   xen/arch/arm/arm64/entry.S |  4 ++-
   xen/arch/arm/traps.c   | 71 
++
   2 files changed, 37 insertions(+), 38 deletions(-)

diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index 40d9f3ec8c..9eafae516b 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -147,7 +147,7 @@

   .if \hyp == 0 /* Guest mode */

-bl  leave_hypervisor_tail /* Disables interrupts on return */
+bl  leave_hypervisor_to_guest /* Disables interrupts on return */

   exit_guest \compat

@@ -175,6 +175,8 @@
   SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
   msr daifclr, \iflags
   mov x0, sp

Looks like this mov can be removed (see commend below).


+bl  enter_hypervisor_from_guest
+mov x0, sp
   bl  do_trap_\trap
   1:
   exithyp=0, compat=\compat
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index a3b961bd06..20ba34ec91 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2006,47 +2006,46 @@ static inline bool needs_ssbd_flip(struct vcpu *v)
cpu_require_ssbd_mitigation();
   }

-static void enter_hypervisor_head(struct cpu_user_regs *regs)
+/*
+ * Actions that needs to be done after exiting the guest and before any
+ * request from it is handled.

Maybe it is me only, but the phrasing is confusing. I had to read it two
times before I get it. What about "Actions that needs to be done when
raising exception level"? Or maybe "Actions that needs to be done when
switching from guest to hypervisor mode" ?


Is it a suggestion to replace the full sentence or just the first
before (i.e. before 'and')?

This is a suggestion for the first part.


How about:

"Actions that needs to be done after entering the hypervisor from the guest and 
before we handle any request."







+ */
+void enter_hypervisor_from_guest(struct cpu_user_regs *regs)

With the guest_mode(regs) check removal , this function does not use regs
anymore.


I have nearly done it while working on the series, but then I thought
that it would be better keep all the functions called from the entry
path in assembly similar.

This can save one assembly instruction in the entry path. But I'm not
sure if it is worth it. So it is up to you.


My concern is user may decide to use guest_cpu_user_regs() when the 'regs' 
parameter could have been used. But I guess, we can notice it during review.


So I will drop it.

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH RFC for-4.13 04/10] xen/arm: Ensure the SSBD workaround is re-enabled right after exiting a guest

[Volodymyr Babchuk]

Julien Grall writes:

> On 27/09/2019 12:56, Volodymyr Babchuk wrote:
>>
>> Julien,
>
> Hi...
>
>>
>> Julien Grall writes:
>>
>>> At the moment, SSBD workaround is re-enabled for Xen after interrupts
>>> are unmasked. This means we may end up to execute some part of the
>>> hypervisor if an interrupt is received before the workaround is
>>> re-enabled.
>>>
>>> As the rest of enter_hypervisor_from_guest() does not require to have
>>> interrupts masked, the function is now split in two parts:
>>>  1) enter_hypervisor_from_guest_noirq() called with interrupts
>>> masked.
>> I'm okay with this approach, but I don't like name for
>> enter_hypervisor_from_guest_noirq(). Right now it is doing exactly one
>> thing - mitigates SSBD. So, maybe more appropriate name will be
>> something like "mitigate_ssbd()" ?
>
> If I wanted to call it mitigate_ssbd() I would have implemented
> completely differently. The reason it is like that is because we may
> need more code to be added here in the future (I have Andrii's series
> in mind). So I would rather avoid a further renaming later on and some
> rework.
Fair enough

>
> Regarding the name, this is a split of
> enter_hypervisor_from_guest(). Hence, why the first path is the
> same. The noirq merely help the user to know what to expect. This is
> better of yet an __ version. Feel free to suggest a better suffix.
I'm bad at naming things :)

I understand that is two halves of one function. But func_name_noirq()
pattern is widely used for other case: when we have func_name_noirq()
function and some func_name() that disables interrupts like this:

void func_name()
{
disable_irqs();
func_name_noirq();
enable_irqs();
}

I like principle of least surprise, so it is better to use some other
naming pattern there.

maybe something like enter_hypervisor_from_guest_pt1() and
enter_hypervisor_from_guest_pt2()?

Or maybe, we should not split the function at all? Instead, we enable
interrupts right in the middle of it.

>
>>
>>>  2) enter_hypervisor_from_guest() called with interrupts unmasked.
>>>
>>> Note that while enter_hypervisor_from_guest_noirq() does not use the
>>> on-stack context registers, it is still passed as parameter to match the
>>> rest of the C functions called from the entry path.
>> As I pointed in the previous email, enter_hypervisor_from_guest() does
>> not use on-stack registers as well.
>
> I am well aware of this, hence my comment here in the commit message
> ;). The reason it is like that is because I wanted to keep the
> prototype the same for all functions called from the entry path (this
> includes do_trap_*).
Let's continue those discussion in the other thread.
[...]

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

Re: [Xen-devel] [PATCH V5] xen/arm: Restrict "p2m_ipa_bits" according to the IOMMU requirements

[Julien Grall]

On 27/09/2019 13:35, Julien Grall wrote:

Hi,

On 27/09/2019 12:57, Oleksandr Tyshchenko wrote:

From: Oleksandr Tyshchenko 

There is a strict requirement for the IOMMU which wants to share
the P2M table with the CPU. The IOMMU's Stage-2 input size must be equal
to the P2M IPA size. It is not a problem when the IOMMU can support
all values the CPU supports. In that case, the IOMMU driver would just
use any "p2m_ipa_bits" value as is. But, there are cases when not.

In order to make P2M sharing possible on the platforms which
IOMMUs have a limitation in maximum Stage-2 input size introduce
the following logic.

First initialize the IOMMU subsystem and gather requirements regarding
the maximum IPA bits supported by each IOMMU device to figure out
the minimum value among them. In the P2M code, take into the account
the IOMMU requirements and choose suitable "pa_range" according
to the restricted "p2m_ipa_bits".

Signed-off-by: Oleksandr Tyshchenko 
CC: Julien Grall 


Reviewed-by: Julien Grall 


Committed now. Thank you!

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH V5] xen/arm: Restrict "p2m_ipa_bits" according to the IOMMU requirements

[Julien Grall]

Hi,

On 27/09/2019 12:57, Oleksandr Tyshchenko wrote:

From: Oleksandr Tyshchenko 

There is a strict requirement for the IOMMU which wants to share
the P2M table with the CPU. The IOMMU's Stage-2 input size must be equal
to the P2M IPA size. It is not a problem when the IOMMU can support
all values the CPU supports. In that case, the IOMMU driver would just
use any "p2m_ipa_bits" value as is. But, there are cases when not.

In order to make P2M sharing possible on the platforms which
IOMMUs have a limitation in maximum Stage-2 input size introduce
the following logic.

First initialize the IOMMU subsystem and gather requirements regarding
the maximum IPA bits supported by each IOMMU device to figure out
the minimum value among them. In the P2M code, take into the account
the IOMMU requirements and choose suitable "pa_range" according
to the restricted "p2m_ipa_bits".

Signed-off-by: Oleksandr Tyshchenko 
CC: Julien Grall 


Reviewed-by: Julien Grall 

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH RFC for-4.13 09/10] xen/arm: asm: Replace use of ALTERNATIVE with alternative_if

[Julien Grall]

Hi,

On 27/09/2019 13:11, Volodymyr Babchuk wrote:



Julien Grall writes:


Using alternative_if makes the code a bit more streamlined.

Take the opportunity to use the new auto-nop infrastructure to avoid
counting the number of nop in the else part for arch/arm/arm64/entry.S

Signed-off-by: Julien Grall 

---
 This is pretty much a matter of taste, but at least for arm64 this
 allows us to use the auto-nop infrastructure. So the arm32 is more
 to keep inline with arm64.
---
  xen/arch/arm/arm32/entry.S | 9 ++---
  xen/arch/arm/arm64/entry.S | 8 +---
  2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S
index 0b4cd19abd..1428cd3583 100644
--- a/xen/arch/arm/arm32/entry.S
+++ b/xen/arch/arm/arm32/entry.S
@@ -65,9 +65,12 @@ save_guest_regs:
   * If the SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT has been set in the cpu
   * feature, the checking of pending SErrors will be skipped.
   */
-ALTERNATIVE("nop",
-"b skip_check",
-SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
+alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
+nop
+alternative_else
+b   skip_check
+alternative_endif
+

for the arm32 code you can have my r-b:
Reviewed-By: Volodymyr Babchuk 


  /*
   * Start to check pending virtual abort in the gap of Guest -> HYP
   * world switch.
diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index 458d12f188..91cf6ee6f4 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -170,9 +170,11 @@
   * is not set. If a vSError took place, the initial exception will be
   * skipped. Exit ASAP
   */
-ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
-"nop; nop",
-SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
+alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
+bl  check_pending_vserror
+cbnzx0, 1f
+alternative_else_nop_endif
+

You asked other people to do not introduce new code in one patch and
rewrite it in the following patch. But there you are doing exactly the
same.


This is a fairly borderline comment knowing that I usually don't request 
clean-up and code consolidation in the same patch.



I believe, it is possible to move all "alternative" patches to the
very beginning of the patch series and only then introduce macro
guest_vector.


For a first, the first patch is definitely not new code. This is code 
consolidation and therefore I don't tend to mix the two for clarity. So this 
should have been a patch before the first patch.


Secondly, the first 4 patches are candidate for backport. The rest of the series 
would be good to backport but I am not aware of a critical issue in previous Xen 
release to strongly push for it.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH] iommu: fix PVH dom0 settings

[Roger Pau Monné]

On Fri, Sep 27, 2019 at 12:45:54PM +0100, Paul Durrant wrote:
> PVH dom0 must operate with the iommu settings in 'strict' mode i.e. only the
> domain's own pages will be mapped in the IOMMU. The check_hwdom_reqs() is
> supposed to ensure this. Unfortunately the test for a PVH dom0 is made
> using paging_mode_translate() and, when commit f89f5558 "remove late
> (on-demand) construction of IOMMU page tables" moved the call of
> check_hwdom_reqs() from iommu_hwdom_init() to iommu_domain_init(), that
> test became ineffective (because iommu_domain_init() is called before
> paging_enable()).
> 
> This patch replaces the test of paging_mode_translate() with a test of
> hap_enabled(), and also verifies 'strict' mode is turned on in
> arch_iommu_check_autotranslated_hwdom().
> 
> Signed-off-by: Paul Durrant 
> Reported-by: Roger Pau Monne 
> ---
> Cc: Jan Beulich 
> ---
>  xen/drivers/passthrough/iommu.c | 6 +++---
>  xen/drivers/passthrough/x86/iommu.c | 3 +++
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
> index 2733b320ec..8b550f909b 100644
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -154,13 +154,13 @@ custom_param("dom0-iommu", parse_dom0_iommu_param);
>  
>  static void __hwdom_init check_hwdom_reqs(struct domain *d)
>  {
> -if ( iommu_hwdom_none || !paging_mode_translate(d) )
> +if ( iommu_hwdom_none || !hap_enabled(d) )

Since dom0 PVH can also be used with shadow paging (not sure how
useful that is TBH), I'm not sure explicitly checking for hap is fine.
What about using is_hvm_domain instead?

That ought to cover both shadow and hap, and a classic PV dom0 won't
be affected by it (which is the intention).

Thanks, Roger.

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

Re: [Xen-devel] [PATCH v7.1 4/4] xen/x86: Allow stubdom access to irq created for msi.

[Jan Beulich]

On 26.09.2019 06:05, Marek Marczykowski-Górecki  wrote:
> Stubdomains need to be given sufficient privilege over the guest which it
> provides emulation for in order for PCI passthrough to work correctly.
> When a HVM domain try to enable MSI, QEMU in stubdomain calls
> PHYSDEVOP_map_pirq, but later it needs to call XEN_DOMCTL_bind_pt_irq as
> part of xc_domain_update_msi_irq. Give the stubdomain enough permissions
> over the mapped interrupt in order to bind it successfully to it's
> target domain.
> 
> This is not needed for PCI INTx, because IRQ in that case is known
> beforehand and the stubdomain is given permissions over this IRQ by
> libxl__device_pci_add (there's a do_pci_add against the stubdomain).
> 
> create_irq() already grant IRQ access to hardware_domain, with
> assumption the device model lives there.
> Modify create_irq() to take additional parameter, whether to grant
> permissions to the domain creating the IRQ, which may be dom0 or a
> stubdomain. Do this instead of granting access always to
> hardware_domain. Save ID of the domain given permission, to revoke it in
> destroy_irq() - easier and cleaner than replaying logic of create_irq()
> parameter. Use domid instead of actual reference to the domain,
> because it might get destroyed before destroying IRQ (stubdomain is
> destroyed before its target domain). And it is not an issue,
> because IRQ permissions live within domain structure, so destroying
> a domain also implicitly revoke the permission.  Potential domid
> reuse is detected by checking if that domain does have permission
> over the IRQ being destroyed.
> 
> Then, adjust all callers to provide the parameter. In case of Xen
> internal allocations, set it to false, but for domain accessible
> interrupt set it to true.
> 
> Inspired by 
> https://github.com/OpenXT/xenclient-oe/blob/5e0e7304a5a3c75ef01240a1e3673665b2aaf05e/recipes-extended/xen/files/stubdomain-msi-irq-access.patch
>  by Eric Chanudet .
> 
> Signed-off-by: Simon Gaiser 
> Signed-off-by: Marek Marczykowski-Górecki 
> Reviewed-by: Roger Pau Monné 

Acked-by: Jan Beulich 
with a couple of cosmetic things addressed, which I'll do while
committing.

Jan

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

Re: [Xen-devel] [PATCH RFC for-4.13 03/10] xen/arm: traps: Rework entry/exit from the guest path

[Volodymyr Babchuk]

Julien Grall writes:

> On 27/09/2019 12:45, Volodymyr Babchuk wrote:
>>
>> Julien,
>
> Hi...
>
>> Julien Grall writes:
>>
>>> At the moment, enter_hypervisor_head() and leave_hypervisor_tail() are
>>> used to deal with actions to be done before/after any guest request is
>>> handled.
>>>
>>> While they are meant to work in pair, the former is called for most of
>>> the traps, including traps from the same exception level (i.e.
>>> hypervisor) whilst the latter will only be called when returning to the
>>> guest.
>>>
>>> As pointed out, the enter_hypervisor_head() is not called from all the
>>> traps, so this makes potentially difficult to extend it for the dealing
>>> with same exception level.
>>>
>>> Furthermore, some assembly only path will require to call
>>> enter_hypervisor_tail(). So the function is now directly call by
>>> assembly in for guest vector only. This means that the check whether we
>>> are called in a guest trap can now be removed.
>>>
>>> Take the opportunity to rename enter_hypervisor_tail() and
>>> leave_hypervisor_tail() to something more meaningful and document them.
>>> This should help everyone to understand the purpose of the two
>>> functions.
>>>
>>> Signed-off-by: Julien Grall 
>>>
>>> ---
>>>
>>> I haven't done the 32-bits part yet. I wanted to gather feedback before
>>> looking in details how to integrate that with Arm32.
>> I'm looking at patches one by one and it is looking okay so far.
>>
>>
>>> ---
>>>   xen/arch/arm/arm64/entry.S |  4 ++-
>>>   xen/arch/arm/traps.c   | 71 
>>> ++
>>>   2 files changed, 37 insertions(+), 38 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
>>> index 40d9f3ec8c..9eafae516b 100644
>>> --- a/xen/arch/arm/arm64/entry.S
>>> +++ b/xen/arch/arm/arm64/entry.S
>>> @@ -147,7 +147,7 @@
>>>
>>>   .if \hyp == 0 /* Guest mode */
>>>
>>> -bl  leave_hypervisor_tail /* Disables interrupts on return */
>>> +bl  leave_hypervisor_to_guest /* Disables interrupts on return 
>>> */
>>>
>>>   exit_guest \compat
>>>
>>> @@ -175,6 +175,8 @@
>>>   SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
>>>   msr daifclr, \iflags
>>>   mov x0, sp
>> Looks like this mov can be removed (see commend below).
>>
>>> +bl  enter_hypervisor_from_guest
>>> +mov x0, sp
>>>   bl  do_trap_\trap
>>>   1:
>>>   exithyp=0, compat=\compat
>>> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
>>> index a3b961bd06..20ba34ec91 100644
>>> --- a/xen/arch/arm/traps.c
>>> +++ b/xen/arch/arm/traps.c
>>> @@ -2006,47 +2006,46 @@ static inline bool needs_ssbd_flip(struct vcpu *v)
>>>cpu_require_ssbd_mitigation();
>>>   }
>>>
>>> -static void enter_hypervisor_head(struct cpu_user_regs *regs)
>>> +/*
>>> + * Actions that needs to be done after exiting the guest and before any
>>> + * request from it is handled.
>> Maybe it is me only, but the phrasing is confusing. I had to read it two
>> times before I get it. What about "Actions that needs to be done when
>> raising exception level"? Or maybe "Actions that needs to be done when
>> switching from guest to hypervisor mode" ?
>
> Is it a suggestion to replace the full sentence or just the first
> before (i.e. before 'and')?
This is a suggestion for the first part.

>>
>>> + */
>>> +void enter_hypervisor_from_guest(struct cpu_user_regs *regs)
>> With the guest_mode(regs) check removal , this function does not use regs
>> anymore.
>
> I have nearly done it while working on the series, but then I thought
> that it would be better keep all the functions called from the entry
> path in assembly similar.
This can save one assembly instruction in the entry path. But I'm not
sure if it is worth it. So it is up to you.

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

Re: [Xen-devel] [PATCH RFC for-4.13 04/10] xen/arm: Ensure the SSBD workaround is re-enabled right after exiting a guest

[Julien Grall]

On 27/09/2019 12:56, Volodymyr Babchuk wrote:


Julien,


Hi...



Julien Grall writes:


At the moment, SSBD workaround is re-enabled for Xen after interrupts
are unmasked. This means we may end up to execute some part of the
hypervisor if an interrupt is received before the workaround is
re-enabled.

As the rest of enter_hypervisor_from_guest() does not require to have
interrupts masked, the function is now split in two parts:
 1) enter_hypervisor_from_guest_noirq() called with interrupts
masked.

I'm okay with this approach, but I don't like name for
enter_hypervisor_from_guest_noirq(). Right now it is doing exactly one
thing - mitigates SSBD. So, maybe more appropriate name will be
something like "mitigate_ssbd()" ?


If I wanted to call it mitigate_ssbd() I would have implemented completely 
differently. The reason it is like that is because we may need more code to be 
added here in the future (I have Andrii's series in mind). So I would rather 
avoid a further renaming later on and some rework.


Regarding the name, this is a split of enter_hypervisor_from_guest(). Hence, why 
the first path is the same. The noirq merely help the user to know what to 
expect. This is better of yet an __ version. Feel free to suggest a better suffix.





 2) enter_hypervisor_from_guest() called with interrupts unmasked.

Note that while enter_hypervisor_from_guest_noirq() does not use the
on-stack context registers, it is still passed as parameter to match the
rest of the C functions called from the entry path.

As I pointed in the previous email, enter_hypervisor_from_guest() does
not use on-stack registers as well.


I am well aware of this, hence my comment here in the commit message ;). The 
reason it is like that is because I wanted to keep the prototype the same for 
all functions called from the entry path (this includes do_trap_*).


[...]




---
  xen/arch/arm/arm64/entry.S |  2 ++
  xen/arch/arm/traps.c   | 16 +---
  2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index 9eafae516b..458d12f188 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -173,6 +173,8 @@
  ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
  "nop; nop",
  SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
+mov x0, sp
+bl  enter_hypervisor_from_guest_noirq
  msr daifclr, \iflags
  mov x0, sp
  bl  enter_hypervisor_from_guest
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 20ba34ec91..5848dd8399 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2007,16 +2007,26 @@ static inline bool needs_ssbd_flip(struct vcpu *v)
  }

  /*
- * Actions that needs to be done after exiting the guest and before any
- * request from it is handled.
+ * Actions that needs to be done after exiting the guest and before the
+ * interrupts are unmasked.
   */
-void enter_hypervisor_from_guest(struct cpu_user_regs *regs)
+void enter_hypervisor_from_guest_noirq(struct cpu_user_regs *regs)
  {
  struct vcpu *v = current;

  /* If the guest has disabled the workaround, bring it back on. */
  if ( needs_ssbd_flip(v) )
  arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 1, NULL);
+}
+
+/*
+ * Actions that needs to be done after exiting the guest and before any
+ * request from it is handled. Depending on the exception trap, this may
+ * be called with interrupts unmasked.
+ */
+void enter_hypervisor_from_guest(struct cpu_user_regs *regs)
+{
+struct vcpu *v = current;

  /*
   * If we pended a virtual abort, preserve it until it gets cleared.



--
Volodymyr Babchuk at EPAM



Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH RFC for-4.13 03/10] xen/arm: traps: Rework entry/exit from the guest path

[Julien Grall]

On 27/09/2019 12:45, Volodymyr Babchuk wrote:


Julien,


Hi...


Julien Grall writes:


At the moment, enter_hypervisor_head() and leave_hypervisor_tail() are
used to deal with actions to be done before/after any guest request is
handled.

While they are meant to work in pair, the former is called for most of
the traps, including traps from the same exception level (i.e.
hypervisor) whilst the latter will only be called when returning to the
guest.

As pointed out, the enter_hypervisor_head() is not called from all the
traps, so this makes potentially difficult to extend it for the dealing
with same exception level.

Furthermore, some assembly only path will require to call
enter_hypervisor_tail(). So the function is now directly call by
assembly in for guest vector only. This means that the check whether we
are called in a guest trap can now be removed.

Take the opportunity to rename enter_hypervisor_tail() and
leave_hypervisor_tail() to something more meaningful and document them.
This should help everyone to understand the purpose of the two
functions.

Signed-off-by: Julien Grall 

---

I haven't done the 32-bits part yet. I wanted to gather feedback before
looking in details how to integrate that with Arm32.

I'm looking at patches one by one and it is looking okay so far.



---
  xen/arch/arm/arm64/entry.S |  4 ++-
  xen/arch/arm/traps.c   | 71 ++
  2 files changed, 37 insertions(+), 38 deletions(-)

diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index 40d9f3ec8c..9eafae516b 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -147,7 +147,7 @@

  .if \hyp == 0 /* Guest mode */

-bl  leave_hypervisor_tail /* Disables interrupts on return */
+bl  leave_hypervisor_to_guest /* Disables interrupts on return */

  exit_guest \compat

@@ -175,6 +175,8 @@
  SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
  msr daifclr, \iflags
  mov x0, sp

Looks like this mov can be removed (see commend below).


+bl  enter_hypervisor_from_guest
+mov x0, sp
  bl  do_trap_\trap
  1:
  exithyp=0, compat=\compat
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index a3b961bd06..20ba34ec91 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2006,47 +2006,46 @@ static inline bool needs_ssbd_flip(struct vcpu *v)
   cpu_require_ssbd_mitigation();
  }

-static void enter_hypervisor_head(struct cpu_user_regs *regs)
+/*
+ * Actions that needs to be done after exiting the guest and before any
+ * request from it is handled.

Maybe it is me only, but the phrasing is confusing. I had to read it two
times before I get it. What about "Actions that needs to be done when
raising exception level"? Or maybe "Actions that needs to be done when
switching from guest to hypervisor mode" ?


Is it a suggestion to replace the full sentence or just the first before (i.e. 
before 'and')?





+ */
+void enter_hypervisor_from_guest(struct cpu_user_regs *regs)

With the guest_mode(regs) check removal , this function does not use regs
anymore.


I have nearly done it while working on the series, but then I thought that it 
would be better keep all the functions called from the entry path in assembly 
similar.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH RFC for-4.13 09/10] xen/arm: asm: Replace use of ALTERNATIVE with alternative_if

[Volodymyr Babchuk]

Julien Grall writes:

> Using alternative_if makes the code a bit more streamlined.
>
> Take the opportunity to use the new auto-nop infrastructure to avoid
> counting the number of nop in the else part for arch/arm/arm64/entry.S
>
> Signed-off-by: Julien Grall 
>
> ---
> This is pretty much a matter of taste, but at least for arm64 this
> allows us to use the auto-nop infrastructure. So the arm32 is more
> to keep inline with arm64.
> ---
>  xen/arch/arm/arm32/entry.S | 9 ++---
>  xen/arch/arm/arm64/entry.S | 8 +---
>  2 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S
> index 0b4cd19abd..1428cd3583 100644
> --- a/xen/arch/arm/arm32/entry.S
> +++ b/xen/arch/arm/arm32/entry.S
> @@ -65,9 +65,12 @@ save_guest_regs:
>   * If the SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT has been set in the cpu
>   * feature, the checking of pending SErrors will be skipped.
>   */
> -ALTERNATIVE("nop",
> -"b skip_check",
> -SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
> +alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
> +nop
> +alternative_else
> +b   skip_check
> +alternative_endif
> +
for the arm32 code you can have my r-b:
Reviewed-By: Volodymyr Babchuk 

>  /*
>   * Start to check pending virtual abort in the gap of Guest -> HYP
>   * world switch.
> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
> index 458d12f188..91cf6ee6f4 100644
> --- a/xen/arch/arm/arm64/entry.S
> +++ b/xen/arch/arm/arm64/entry.S
> @@ -170,9 +170,11 @@
>   * is not set. If a vSError took place, the initial exception will be
>   * skipped. Exit ASAP
>   */
> -ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
> -"nop; nop",
> -SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
> +alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
> +bl  check_pending_vserror
> +cbnzx0, 1f
> +alternative_else_nop_endif
> +
You asked other people to do not introduce new code in one patch and
rewrite it in the following patch. But there you are doing exactly the
same. I believe, it is possible to move all "alternative" patches to the
very beginning of the patch series and only then introduce macro
guest_vector.

>  mov x0, sp
>  bl  enter_hypervisor_from_guest_noirq
>  msr daifclr, \iflags


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

Re: [Xen-devel] [PATCH] iommu: fix PVH dom0 settings

[Jürgen Groß]

On 27.09.19 14:02, Jan Beulich wrote:

On 27.09.2019 13:45, Paul Durrant wrote:

PVH dom0 must operate with the iommu settings in 'strict' mode i.e. only the
domain's own pages will be mapped in the IOMMU. The check_hwdom_reqs() is
supposed to ensure this. Unfortunately the test for a PVH dom0 is made
using paging_mode_translate() and, when commit f89f5558 "remove late
(on-demand) construction of IOMMU page tables" moved the call of
check_hwdom_reqs() from iommu_hwdom_init() to iommu_domain_init(), that
test became ineffective (because iommu_domain_init() is called before
paging_enable()).

This patch replaces the test of paging_mode_translate() with a test of
hap_enabled(), and also verifies 'strict' mode is turned on in
arch_iommu_check_autotranslated_hwdom().

Signed-off-by: Paul Durrant 
Reported-by: Roger Pau Monne 


Oh, and I guess you've also meant to Cc Jürgen for a release ack
(now done)?


Thanks for the Cc!

Release-acked-by: Juergen Gross 


Juergen

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

Re: [Xen-devel] [PATCH] iommu: fix PVH dom0 settings

[Jan Beulich]

On 27.09.2019 13:45, Paul Durrant wrote:
> PVH dom0 must operate with the iommu settings in 'strict' mode i.e. only the
> domain's own pages will be mapped in the IOMMU. The check_hwdom_reqs() is
> supposed to ensure this. Unfortunately the test for a PVH dom0 is made
> using paging_mode_translate() and, when commit f89f5558 "remove late
> (on-demand) construction of IOMMU page tables" moved the call of
> check_hwdom_reqs() from iommu_hwdom_init() to iommu_domain_init(), that
> test became ineffective (because iommu_domain_init() is called before
> paging_enable()).
> 
> This patch replaces the test of paging_mode_translate() with a test of
> hap_enabled(), and also verifies 'strict' mode is turned on in
> arch_iommu_check_autotranslated_hwdom().
> 
> Signed-off-by: Paul Durrant 
> Reported-by: Roger Pau Monne 

Oh, and I guess you've also meant to Cc Jürgen for a release ack
(now done)?

Jan

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

Re: [Xen-devel] [PATCH] iommu: fix PVH dom0 settings

[Jan Beulich]

On 27.09.2019 13:45, Paul Durrant wrote:
> PVH dom0 must operate with the iommu settings in 'strict' mode i.e. only the
> domain's own pages will be mapped in the IOMMU. The check_hwdom_reqs() is
> supposed to ensure this. Unfortunately the test for a PVH dom0 is made
> using paging_mode_translate() and, when commit f89f5558 "remove late
> (on-demand) construction of IOMMU page tables" moved the call of
> check_hwdom_reqs() from iommu_hwdom_init() to iommu_domain_init(), that
> test became ineffective (because iommu_domain_init() is called before
> paging_enable()).
> 
> This patch replaces the test of paging_mode_translate() with a test of
> hap_enabled(), and also verifies 'strict' mode is turned on in
> arch_iommu_check_autotranslated_hwdom().
> 
> Signed-off-by: Paul Durrant 
> Reported-by: Roger Pau Monne 

Reviewed-by: Jan Beulich 

Nit: Conventionally your two tags should be the other way around,
to represent events chronologically.

Jan

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

Re: [Xen-devel] [PATCH RFC for-4.13 06/10] xen/arm: Move ARCH_PATCH_INSN_SIZE out of the header livepatch.h

[Ross Lagerwall]

On 9/26/19 7:38 PM, Julien Grall wrote:

At the moment, ARCH_PATCH_INSN_SIZE is defined in the header
livepatch.h. However, this is also used in the alternative code.

Rather than including livepatch.h just for using the define, move it in
the header insn.h which seems more suitable.


Reviewed-by: Ross Lagerwall 

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

[Xen-devel] [PATCH V5] xen/arm: Restrict "p2m_ipa_bits" according to the IOMMU requirements

[Oleksandr Tyshchenko]

From: Oleksandr Tyshchenko 

There is a strict requirement for the IOMMU which wants to share
the P2M table with the CPU. The IOMMU's Stage-2 input size must be equal
to the P2M IPA size. It is not a problem when the IOMMU can support
all values the CPU supports. In that case, the IOMMU driver would just
use any "p2m_ipa_bits" value as is. But, there are cases when not.

In order to make P2M sharing possible on the platforms which
IOMMUs have a limitation in maximum Stage-2 input size introduce
the following logic.

First initialize the IOMMU subsystem and gather requirements regarding
the maximum IPA bits supported by each IOMMU device to figure out
the minimum value among them. In the P2M code, take into the account
the IOMMU requirements and choose suitable "pa_range" according
to the restricted "p2m_ipa_bits".

Signed-off-by: Oleksandr Tyshchenko 
CC: Julien Grall 

---
Please note, this patch wasn't checked for the SMMU.

For IPMMU it works well:

w/ IOMMU:
(XEN) I/O virtualisation enabled
(XEN)  - Dom0 mode: Relaxed
(XEN) Interrupt remapping enabled
(XEN) P2M: 40-bit IPA with 40-bit PA and 8-bit VMID
(XEN) P2M: 3 levels with order-1 root, VTCR 0x80023558
(XEN) Adding cpu 0 to runqueue 0

w/o IOMMU:
(XEN) I/O virtualisation disabled
(XEN) P2M: 44-bit IPA with 44-bit PA and 8-bit VMID
(XEN) P2M: 4 levels with order-0 root, VTCR 0x80043594


Changes V4 [4] -> V5:
   - [SMMU] Use p2m_ipa_bits instead of smmu->s2_input_size for
 the TTBCR calculation

Changes RFC V3 [3] -> V4 [4]:
   - Move check for p2m_ipa_bits to be at least 40 bit
 under #ifdef CONFIG_ARM_32
   - Reword the "panic" message

Changes RFC V2 [2] -> RFC V3 [3]:
   - Check in setup_virt_paging() that the "restricted"
 P2M IPA size is at least 40-bit
   - Modify logic in setup_virt_paging() a bit to make it
 "IOMMU-agnostic"
   - Clarify comments in code, add some explanations
   - Avoid using the term "IOMMU" in P2M code where possible

Changes RFC V1 [1] -> RFC V2 [2]:
   - Don't update p2m_ipa_bits by the IOMMU drivers directly,
 introduce p2m_restrict_ipa_bits()
   - Clarify patch subject/description
   - Add more comments to code
   - Check for equivalent "pabits" in setup_virt_paging()
   - Remove ASSERTs from the SMMU and IPMMU drivers

[1] https://lists.xenproject.org/archives/html/xen-devel/2019-08/msg02078.html
[2] https://lists.xenproject.org/archives/html/xen-devel/2019-08/msg02237.html
[3] https://lists.xenproject.org/archives/html/xen-devel/2019-09/msg00973.html
[4] https://lists.xenproject.org/archives/html/xen-devel/2019-09/msg02304.html
---
 xen/arch/arm/p2m.c   | 41 
 xen/arch/arm/setup.c |  9 +--
 xen/drivers/passthrough/arm/ipmmu-vmsa.c | 18 ++
 xen/drivers/passthrough/arm/smmu.c   | 17 +++--
 xen/include/asm-arm/p2m.h|  9 +++
 5 files changed, 63 insertions(+), 31 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index e859763..5c7504e 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -34,7 +34,11 @@ static unsigned int __read_mostly max_vmid = MAX_VMID_8_BIT;
 
 #define P2M_ROOT_PAGES(1mm64.pa_range;
+
+/*
+ * Restrict "p2m_ipa_bits" if needed. As P2M table is always configured
+ * with IPA bits == PA bits, compare against "pabits".
+ */
+if ( pa_range_info[info->mm64.pa_range].pabits < p2m_ipa_bits )
+p2m_ipa_bits = pa_range_info[info->mm64.pa_range].pabits;
 
 /* Set a flag if the current cpu does not support 16 bit VMIDs. */
 if ( info->mm64.vmid_bits != MM64_VMID_16_BITS_SUPPORT )
@@ -2003,6 +2026,16 @@ void __init setup_virt_paging(void)
 if ( !vmid_8_bit )
 max_vmid = MAX_VMID_16_BIT;
 
+/* Choose suitable "pa_range" according to the resulted "p2m_ipa_bits". */
+for ( i = 0; i < ARRAY_SIZE(pa_range_info); i++ )
+{
+if ( p2m_ipa_bits == pa_range_info[i].pabits )
+{
+pa_range = i;
+break;
+}
+}
+
 /* pa_range is 4 bits, but the defined encodings are only 3 bits */
 if ( pa_range >= ARRAY_SIZE(pa_range_info) || 
!pa_range_info[pa_range].pabits )
 panic("Unknown encoding of ID_AA64MMFR0_EL1.PARange %x\n", pa_range);
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index fca7e68..790eab9 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -928,12 +928,17 @@ void __init start_xen(unsigned long boot_phys_offset,
 printk("Brought up %ld CPUs\n", (long)num_online_cpus());
 /* TODO: smp_cpus_done(); */
 
-setup_virt_paging();
-
+/*
+ * The IOMMU subsystem must be initialized before P2M as we need
+ * to gather requirements regarding the maximum IPA bits supported by
+ * each IOMMU device.
+ */
 rc = iommu_setup();
 if ( !iommu_enabled && rc != -ENODEV )
 panic("Couldn't 

Re: [Xen-devel] [PATCH RFC for-4.13 04/10] xen/arm: Ensure the SSBD workaround is re-enabled right after exiting a guest

[Volodymyr Babchuk]

Julien,

Julien Grall writes:

> At the moment, SSBD workaround is re-enabled for Xen after interrupts
> are unmasked. This means we may end up to execute some part of the
> hypervisor if an interrupt is received before the workaround is
> re-enabled.
>
> As the rest of enter_hypervisor_from_guest() does not require to have
> interrupts masked, the function is now split in two parts:
> 1) enter_hypervisor_from_guest_noirq() called with interrupts
>masked.
I'm okay with this approach, but I don't like name for
enter_hypervisor_from_guest_noirq(). Right now it is doing exactly one
thing - mitigates SSBD. So, maybe more appropriate name will be
something like "mitigate_ssbd()" ?

> 2) enter_hypervisor_from_guest() called with interrupts unmasked.
>
> Note that while enter_hypervisor_from_guest_noirq() does not use the
> on-stack context registers, it is still passed as parameter to match the
> rest of the C functions called from the entry path.
As I pointed in the previous email, enter_hypervisor_from_guest() does
not use on-stack registers as well.

> Fixes: a7898e4c59 ("xen/arm: Add ARCH_WORKAROUND_2 support for guests")
> Reported-by: Andrii Anisov 
> Signed-off-by: Julien Grall 
>
> ---
>
> Note the Arm32 code has not been changed yet. I am also open on turn
> both enter_hypervisor_from_guest_noirq() and enter_hypervisor_from()
> to functions not taking any parameters.
That would be appropriate in my opinion.

> ---
>  xen/arch/arm/arm64/entry.S |  2 ++
>  xen/arch/arm/traps.c   | 16 +---
>  2 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
> index 9eafae516b..458d12f188 100644
> --- a/xen/arch/arm/arm64/entry.S
> +++ b/xen/arch/arm/arm64/entry.S
> @@ -173,6 +173,8 @@
>  ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
>  "nop; nop",
>  SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
> +mov x0, sp
> +bl  enter_hypervisor_from_guest_noirq
>  msr daifclr, \iflags
>  mov x0, sp
>  bl  enter_hypervisor_from_guest
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 20ba34ec91..5848dd8399 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2007,16 +2007,26 @@ static inline bool needs_ssbd_flip(struct vcpu *v)
>  }
>
>  /*
> - * Actions that needs to be done after exiting the guest and before any
> - * request from it is handled.
> + * Actions that needs to be done after exiting the guest and before the
> + * interrupts are unmasked.
>   */
> -void enter_hypervisor_from_guest(struct cpu_user_regs *regs)
> +void enter_hypervisor_from_guest_noirq(struct cpu_user_regs *regs)
>  {
>  struct vcpu *v = current;
>
>  /* If the guest has disabled the workaround, bring it back on. */
>  if ( needs_ssbd_flip(v) )
>  arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 1, NULL);
> +}
> +
> +/*
> + * Actions that needs to be done after exiting the guest and before any
> + * request from it is handled. Depending on the exception trap, this may
> + * be called with interrupts unmasked.
> + */
> +void enter_hypervisor_from_guest(struct cpu_user_regs *regs)
> +{
> +struct vcpu *v = current;
>
>  /*
>   * If we pended a virtual abort, preserve it until it gets cleared.


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

Re: [Xen-devel] [PATCH RFC for-4.13 07/10] xen/arm: Allow insn.h to be called from assembly

[Volodymyr Babchuk]

Julien Grall writes:

> A follow-up patch will require to include insn.h from assembly code. So
> wee need to protect any C-specific definition to avoid compilation
> error when used in assembly code.
>
> Signed-off-by: Julien Grall 
Reviewed-by: Volodymyr Babchuk 

> ---
>  xen/include/asm-arm/insn.h | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/xen/include/asm-arm/insn.h b/xen/include/asm-arm/insn.h
> index 19277212e1..00391f83f9 100644
> --- a/xen/include/asm-arm/insn.h
> +++ b/xen/include/asm-arm/insn.h
> @@ -1,8 +1,14 @@
>  #ifndef __ARCH_ARM_INSN
>  #define __ARCH_ARM_INSN
>  
> +#ifndef __ASSEMBLY__
> +
>  #include 
>  
> +/*
> + * At the moment, arch-specific headers contain only definition for C
> + * code.
> + */
>  #if defined(CONFIG_ARM_64)
>  # include 
>  #elif defined(CONFIG_ARM_32)
> @@ -11,6 +17,8 @@
>  # error "unknown ARM variant"
>  #endif
>  
> +#endif /* __ASSEMBLY__ */
> +
>  /* On ARM32,64 instructions are always 4 bytes long. */
>  #define ARCH_PATCH_INSN_SIZE 4


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

Re: [Xen-devel] [PATCH RFC for-4.13 06/10] xen/arm: Move ARCH_PATCH_INSN_SIZE out of the header livepatch.h

[Volodymyr Babchuk]

Julien Grall writes:

> At the moment, ARCH_PATCH_INSN_SIZE is defined in the header
> livepatch.h. However, this is also used in the alternative code.
>
> Rather than including livepatch.h just for using the define, move it in
> the header insn.h which seems more suitable.
>
> Signed-off-by: Julien Grall 
Reviewed-by: Volodymyr Babchuk 

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

Re: [Xen-devel] [PATCH RFC for-4.13 05/10] xen/arm: alternative: Remove unused parameter for alternative_if_not_cap

[Volodymyr Babchuk]

Julien Grall writes:

> The macro alternative_if_not_cap is taking two parameters. The second
> parameter is never used and it is hard to see how this can be used
> correctly as it is only protecting the alternative section magic.
>
> Signed-off-by: Julien Grall 
Reviewed-by: Volodymyr Babchuk 

> ---
>  xen/include/asm-arm/alternative.h | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/xen/include/asm-arm/alternative.h 
> b/xen/include/asm-arm/alternative.h
> index dedb6dd001..2830a6da2d 100644
> --- a/xen/include/asm-arm/alternative.h
> +++ b/xen/include/asm-arm/alternative.h
> @@ -116,13 +116,11 @@ int apply_alternatives(const struct alt_instr *start, 
> const struct alt_instr *en
>   * The code that follows this macro will be assembled and linked as
>   * normal. There are no restrictions on this code.
>   */
> -.macro alternative_if_not cap, enable = 1
> - .if \enable
> +.macro alternative_if_not cap
>   .pushsection .altinstructions, "a"
>   altinstruction_entry 661f, 663f, \cap, 662f-661f, 664f-663f
>   .popsection
>  661:
> - .endif
>  .endm
>  
>  /*


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

[Xen-devel] [PATCH] iommu: fix PVH dom0 settings

[Paul Durrant]

PVH dom0 must operate with the iommu settings in 'strict' mode i.e. only the
domain's own pages will be mapped in the IOMMU. The check_hwdom_reqs() is
supposed to ensure this. Unfortunately the test for a PVH dom0 is made
using paging_mode_translate() and, when commit f89f5558 "remove late
(on-demand) construction of IOMMU page tables" moved the call of
check_hwdom_reqs() from iommu_hwdom_init() to iommu_domain_init(), that
test became ineffective (because iommu_domain_init() is called before
paging_enable()).

This patch replaces the test of paging_mode_translate() with a test of
hap_enabled(), and also verifies 'strict' mode is turned on in
arch_iommu_check_autotranslated_hwdom().

Signed-off-by: Paul Durrant 
Reported-by: Roger Pau Monne 
---
Cc: Jan Beulich 
---
 xen/drivers/passthrough/iommu.c | 6 +++---
 xen/drivers/passthrough/x86/iommu.c | 3 +++
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 2733b320ec..8b550f909b 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -154,13 +154,13 @@ custom_param("dom0-iommu", parse_dom0_iommu_param);
 
 static void __hwdom_init check_hwdom_reqs(struct domain *d)
 {
-if ( iommu_hwdom_none || !paging_mode_translate(d) )
+if ( iommu_hwdom_none || !hap_enabled(d) )
 return;
 
-arch_iommu_check_autotranslated_hwdom(d);
-
 iommu_hwdom_passthrough = false;
 iommu_hwdom_strict = true;
+
+arch_iommu_check_autotranslated_hwdom(d);
 }
 
 int iommu_domain_init(struct domain *d, unsigned int opts)
diff --git a/xen/drivers/passthrough/x86/iommu.c 
b/xen/drivers/passthrough/x86/iommu.c
index 47a3e55213..f54805babd 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -85,6 +85,9 @@ void __hwdom_init 
arch_iommu_check_autotranslated_hwdom(struct domain *d)
 {
 if ( !is_iommu_enabled(d) )
 panic("Presently, iommu must be enabled for PVH hardware domain\n");
+
+if ( !iommu_hwdom_strict )
+panic("PVH hardware domain iommu must be set in 'strict' mode\n");
 }
 
 int arch_iommu_domain_init(struct domain *d)
-- 
2.20.1.2.gb21ebb671


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