[Xen-devel] [BUG] panic: "IO-APIC + timer doesn't work" - several people have reproduced

[Aaron Janse]

Hello all,

After attempting to install QubesOS on a new laptop, I've stumbled upon
a group of people with an assortment of laptops but all the same
problem: a Xen panic stating "IO-APIC + timer doesn't work!"

Many of us are on different stages of debugging this, so I'll cite all
our efforts here.

# Affected Xen versions
- 4.0.5-14.fc25
- 4.13.0
- probably other versions

# Affected hardware

- Dell XPS 7390 13" (i7-10710U) [1] [2] [3] [4]
- Dell XPS 7390 2-in-1 [1]
- Surface Laptop 3 Business Edition (i7-1065G7) [5] [6]
- Lenovo ThinkBook 13s (i7-8565U) [7]
- Mini-PcBarebone [8] [9]

[1]: https://www.reddit.com/r/Qubes/comments/edqrab/qubes_and_ice_lake/

[2]: 
https://www.reddit.com/r/Qubes/comments/dfv6jx/panic_on_cpu_0_ioapic_timer_doesnt_work_on_ice/
[3]: https://groups.google.com/forum/#!topic/qubes-users/W8mX-07xNZU
[4]: https://lists.xenproject.org/archives/html/xen-users/2019-12/msg00031.html

[5]: https://lists.xenproject.org/archives/html/xen-users/2019-12/msg00017.html
[6]: https://groups.google.com/forum/#!topic/qubes-users/4iswU7cfJHY

[7]: 
https://www.reddit.com/r/Qubes/comments/clk0eu/help_install_qubes_4_on_lenovo_thinkbook_13s/

[8]: https://groups.google.com/forum/#!topic/qubes-users/PIyz7BEV1mg
[9]: https://archive.is/RuiAD

# Excerpts from boot logs

Qubes on my XPS 7390 13"

(XEN) ..TIMER: vector=0xF0 apic1=0 pin1=2 apic2=-1 pin2=-1
(XEN) ..MP-BIOS bug: 8254 timer not connected to IO-APIC
(XEN) ...trying to set up timer (IRQ0) through 8259A ,,,
(XEN) ...trying to set up timer as Virtual Wire IRQ... failed.
(XEN) ...trying to set up timer as ExtINT IRQ...spurious 8259A interrupt: 
IRQ7.
(XEN) CPU0: no irq handler for vector e7 (IRQ -8)
(XEN) IRQ7 a=0001[0001,] v=60[] t=IO-APIC-edge s=0002
(XEN)  failed :(.
(XEN)
(XEN) ***
(XEN) Panic on CPU 0:
(XEN) IO-APIC + timer doesn't work! Boot with apic_verbosity=debug and send 
a report.  Then try booting with the 'noapic' option
(XEN) ***

Ubuntu (w/out Xen) on my XPS 7390 13"
(full logs: https://pastebin.com/SdRg87F8 https://pastebin.com/E3zCfb35)

[0.00] microcode: microcode updated early to revision 0xca, date = 
2019-10-03
[0.00] Linux version 5.3.0-24-generic (buildd@lgw01-amd64-035) (gcc 
version 9.2.1 20191008 (Ubuntu 9.2.1-9ubuntu2)) #26-Ubuntu SMP Thu Nov 14 
01:33:18 UTC 2019 (Ubuntu 5.3.0-24.26-generic 5.3.10)
[0.00] Command line: BOOT_IMAGE=/boot/vmlinuz-5.3.0-24-generic 
root=UUID=062d0c69-31dc-4c7f-9915-731505fea81b ro quiet splash apic=debug 
vt.handoff=7
[]
[0.188468] x2apic enabled
[0.188492] Switched APIC routing to cluster x2apic.
[0.188493] masked ExtINT on CPU#0
[0.195266] ENABLING IO-APIC IRQs
[0.195267] init IO_APIC IRQs
[]
[0.195633] ..TIMER: vector=0x30 apic1=0 pin1=2 apic2=-1 pin2=-1
[0.213854] clocksource: tsc-early: mask: 0x max_cycles: 
0x170fff30cc4, max_idle_ns: 440795237869 ns
[0.213857] Calibrating delay loop (skipped), value calculated using 
timer frequency.. 3199.92 BogoMIPS (lpj=6399840)
[0.213859] pid_max: default: 32768 minimum: 301
[0.215117] LSM: Security Framework initializing
[0.215124] Yama: becoming mindful.
[0.215172] AppArmor: AppArmor initialized

I'd like to note that Ubuntu, unlike Qubes, doesn't need to try
any `MP-BIOS bug` fallbacks.

# Relevant source code

Xen: 
https://github.com/xen-project/xen/blob/0cd791c499bdc698d14a24050ec56d60b45732e0/xen/arch/x86/io_apic.c#L1923-L1933
Linux: 
https://github.com/torvalds/linux/blob/fd6988496e79a6a4bdb514a4655d2920209eb85d/arch/x86/kernel/apic/io_apic.c#L2185-L2211

# Things that have been tried

Disabling APIC entirely (`noapic x2apic=off`)
- This is avoiding the problem, not fixing it
- QubeOS requires APIC anyway, so this is not an option for many of us

Switching the timer to HPET (via the `clocksource` flag)
- This didn't fix the panic (I've tried `acpi` and `pit`)
- On my XPS 13, this doesn't change any of the timer error output
- Ubuntu works on my laptop using HPET
- 
http://xenbits.xen.org/docs/unstable/misc/xen-command-line.html#clocksource-x86

Updating to 5.4 Linux kernel [77]
- This didn't fix the panic
- https://www.reddit.com/r/Qubes/comments/edqrab/qubes_and_ice_lake/fcak799/

Reproducing the problem on Ubuntu
- I'm able to reproduce Xen crashing, but I'm unable to enable verbose
  logging
- https://lists.xenproject.org/archives/html/xen-users/2019-12/msg00031.html

# More verbose boot logs

I had to type these up by hand so please excuse the lack of detail.

(XEN) [...]
(XEN) CPU0: No irq handler for vector 40 (IRC -2147483648, LAPIC)
(XEN) PCI: MCFG configuration 0: base e000 segment  buses 00 - ff
(XEN) PCI: Not using MCFG for segment  bus 00-ff
(XEN) Intel VT-d iommu 0 supported 

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail REGR. vs. 
145025

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

version targeted for testing:
 xen  3a13ac3ad4d3ef399fe2c85fb09fcb7ab1cdd140
baseline version:
 xen  0cd791c499bdc698d14a24050ec56d60b45732e0

Last test of basis   145025  2019-12-20 13:58:10 Z   10 days
Failing since145058  2019-12-21 07:15:37 Z9 days   24 attempts
Testing same since   145321  2019-12-28 07:51:14 Z2 days7 attempts


People who touched 

Re: [Xen-devel] REGRESSION: Xen 4.13 RC5 fails to bootstrap Dom0 on ARM

[Roman Shaposhnik]

Hi Julien,

On Sun, Dec 29, 2019 at 10:01 AM Julien Grall  wrote:
>
> Hi,
>
> On 21/12/2019 01:37, Roman Shaposhnik wrote:
> > On Thu, Dec 19, 2019 at 4:01 PM Stefano Stabellini
> >  wrote:
> >>
> >> On Thu, 19 Dec 2019, Julien Grall wrote:
> > In fact most of people on Arm are using GRUB rather than EFI directly as
> > this is more friendly to use.
> >
> > Regarding the devicetree, Xen and Linux will completely ignore the
> > memory nodes in Xen if using EFI. This because the EFI memory map will
> > give you an overview of the platform with the EFI regions included.
> 
>  Aha! So in that sense it is a bug in Xen after all, right? (that's what
>  you're
>  referring to when you say you now understand what needs to get fixed).
> >>>
> >>> Yes. The EFI memory map is a list of existing memory with a type 
> >>> associated to
> >>> it (Conventional, BootServiceCodes, MemoryMappedIO...).
> >>>
> >>> The OS/Hypervisor will have to go through them and check which regions are
> >>> usuable. Compare to Linux, Xen has limited itself to only a few types.
> >>>
> >>> However, I think we can be on a par with Linux here.
> >>
> >> I gave a look at the Linux implementation, the interesting bit is
> >> drivers/firmware/efi/arm-init.c:is_usable_memory as far as I can tell.
> >> I also gave a look at the Xen side, which is
> >> xen/arch/arm/efi/efi-boot.h:efi_process_memory_map_bootinfo. As guessed,
> >> the two are not quite the same.
> >>
> >> One of the main differences is that Linux uses as "System RAM" even
> >> regions that were marked as EFI_BOOT_SERVICES_CODE/DATA and
> >> EFI_LOADER_CODE/DATA because they will get freed anyway. Xen doesn't
> >> do that unless map_bs is set.
> >>
> >> I wrote a quick patch to implement the Linux behavior on Xen, only
> >> lightly tested. I can confirm that I see more memory this way. However,
> >> I am not sure we actually want to import the Linux behavior wholesale.
> >>
> >> Anyway, Roman, could you please let me know if this patch solves the
> >> issue?
> >
> > Tried the attached patch -- but it seems I can't boot at all with this. Xen
> > doesn't print anything on the console either.
>
> Thank you for trying the patch. Do you have earlyprintk enabled for the
> hikey board?

No (since I thought it wasn't possible on ARM :-)) but now that you
mentioned it,
I've found this:
 http://xenbits.xenproject.org/docs/4.13-testing/misc/arm/early-printk.txt
and I'd be more than happy to try (hopefully CONFIG_EARLY_PRINTK= hikey960
will do the trick).

> > To Julien's point -- should I reduce the # of types and try again?
>
>  From my understanding, the field Attribute is a series of flag telling
> what the region can support.
>
> So it would be possible to have other flags set at the same time as
> EFI_MEMORY_WC. However, the check in the patch below is an == equal and
> would potentially discard a lot of regions (if not all regions).
>
> In other words...
>
> >>
> >>
> >> diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
> >> index ca655ff003..ad18ff3669 100644
> >> --- a/xen/arch/arm/efi/efi-boot.h
> >> +++ b/xen/arch/arm/efi/efi-boot.h
> >> @@ -149,10 +149,14 @@ static EFI_STATUS __init 
> >> efi_process_memory_map_bootinfo(EFI_MEMORY_DESCRIPTOR *
> >>
> >>   for ( Index = 0; Index < (mmap_size / desc_size); Index++ )
> >>   {
> >> -if ( desc_ptr->Type == EfiConventionalMemory ||
> >> - (!map_bs &&
> >> -  (desc_ptr->Type == EfiBootServicesCode ||
> >> -   desc_ptr->Type == EfiBootServicesData)) )
> >> +if ( desc_ptr->Attribute == EFI_MEMORY_WB &&
>
> ... this should be desc_ptr->Attribute & EFI_MEMORY_WB.
>
> Can you give a spin with this change and see how far you can go?

Aha! That makes much more sense -- will give it a try tomorrow
(in conjunction with earlyprintk)

Thanks,
Roman.

> >> + (desc_ptr->Type == EfiConventionalMemory ||
> >> +  desc_ptr->Type == EfiLoaderCode ||
> >> +  desc_ptr->Type == EfiLoaderData ||
> >> +  desc_ptr->Type == EfiACPIReclaimMemory ||
> >> +  desc_ptr->Type == EfiPersistentMemory ||
> >> +  desc_ptr->Type == EfiBootServicesCode ||
> >> +  desc_ptr->Type == EfiBootServicesData) )
> >>   {
> >>   if ( !meminfo_add_bank(, desc_ptr) )
> >>   {
> >> diff --git a/xen/include/efi/efidef.h b/xen/include/efi/efidef.h
> >> index 86a7e111bf..f46207840f 100644
> >> --- a/xen/include/efi/efidef.h
> >> +++ b/xen/include/efi/efidef.h
> >> @@ -147,6 +147,7 @@ typedef enum {
> >>   EfiMemoryMappedIO,
> >>   EfiMemoryMappedIOPortSpace,
> >>   EfiPalCode,
> >> +EfiPersistentMemory,
> >>   EfiMaxMemoryType
> >>   } EFI_MEMORY_TYPE;
> >>
>
> Cheers,
>
> --
> Julien Grall

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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-freebsd10-i386 14 guest-saverestore  fail REGR. vs. 144861
 test-amd64-i386-freebsd10-amd64 14 guest-saverestore fail REGR. vs. 144861
 test-amd64-i386-xl-qemuu-debianhvm-amd64 13 guest-saverestore fail REGR. vs. 
144861
 test-amd64-amd64-xl-qemuu-ovmf-amd64 13 guest-saverestore fail REGR. vs. 144861
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 guest-saverestore fail 
REGR. vs. 144861
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 13 guest-saverestore fail 
REGR. vs. 144861
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 13 guest-saverestore fail REGR. vs. 
144861
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 guest-saverestore fail 
REGR. vs. 144861
 test-amd64-amd64-xl-qemuu-win7-amd64 13 guest-saverestore fail REGR. vs. 144861
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 13 guest-saverestore fail REGR. 
vs. 144861
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 13 guest-saverestore fail REGR. 
vs. 144861
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 13 guest-saverestore fail 
REGR. vs. 144861
 test-amd64-i386-xl-qemuu-ovmf-amd64 13 guest-saverestore fail REGR. vs. 144861
 test-amd64-i386-xl-qemuu-win7-amd64 13 guest-saverestore fail REGR. vs. 144861
 test-amd64-amd64-xl-qemuu-ws16-amd64 13 guest-saverestore fail REGR. vs. 144861
 test-amd64-i386-xl-qemuu-ws16-amd64 13 guest-saverestore fail REGR. vs. 144861

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-rtds15 guest-saverestore fail in 145206 pass in 145401
 test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail pass in 
145046
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeat  fail pass in 145206

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds 17 guest-start.2  fail in 145206 REGR. vs. 144861

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10   fail  like 144861
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 144861
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 144861
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-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-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  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-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-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
 

Re: [Xen-devel] [PATCH v2 00/20] VM forking

[Tamas K Lengyel]

On Mon, Dec 30, 2019 at 5:20 PM Julien Grall  wrote:
>
> Hi,
>
> On Mon, 30 Dec 2019, 20:49 Tamas K Lengyel,  wrote:
>>
>> On Mon, Dec 30, 2019 at 11:43 AM Julien Grall  wrote:
>> But keep in mind that the "fork-vm" command even with this update
>> would still not produce for you a "fully functional" VM on its own.
>> The user still has to produce a new VM config file, create the new
>> disk, save the QEMU state, etc.
>
>
>  If you fork then the configuration should be very similar. Right?
>
> So why does the user requires to provide a new config rather than the command 
> to update the existing one? To me, it feels this is a call to make mistake 
> when forking.
>
> How is the new config different from the original VM?

The config must be different at least by giving the fork a different
name. That's the minimum and it's enough only if the VM you are
forking has no disk at all. If it has a disk, you also have to update
the config to point to where the new disk is. I'm using LVM snapshots
but you could also use qcow2, or whatever else there is for disk-CoW.
The fork can also have different options enabled than it's parent. For
example in our test-case, the forks have altp2m enabled while the
parent VM doesn't. There could be other options like that someone
might want to enable for the fork(s). If there is networking involved
you likely also have to attach the fork to a new VLAN as to avoid
MAC-address collision on the bridge. So there are quite a lot of
variation possible, hence its better to have the user generate the new
config they want instead of xl coming up with something on its own.

>
> As a side note, I can't see any patch adding documentation.

It's only an experimental feature so adding documentation was not a
priority. The documentation is pretty much in the cover letter. I'm
happy to add its content as a file under docs in a patch (with the
above extra information).

Tamas

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

Re: [Xen-devel] [PATCH v2 00/20] VM forking

[Julien Grall]

Hi,

On Mon, 30 Dec 2019, 20:49 Tamas K Lengyel,  wrote:

> On Mon, Dec 30, 2019 at 11:43 AM Julien Grall  wrote:
> But keep in mind that the "fork-vm" command even with this update
> would still not produce for you a "fully functional" VM on its own.
> The user still has to produce a new VM config file, create the new
> disk, save the QEMU state, etc.


 If you fork then the configuration should be very similar. Right?

So why does the user requires to provide a new config rather than the
command to update the existing one? To me, it feels this is a call to make
mistake when forking.

How is the new config different from the original VM?

As a side note, I can't see any patch adding documentation.

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

Re: [Xen-devel] [PATCH v2 00/20] VM forking

[Tamas K Lengyel]

On Mon, Dec 30, 2019 at 11:43 AM Julien Grall  wrote:
>
> Hi Tamas,
>
> On 30/12/2019 18:15, Tamas K Lengyel wrote:
> > On Mon, Dec 30, 2019 at 10:59 AM Roger Pau Monné  
> > wrote:
> >>
> >> On Thu, Dec 19, 2019 at 08:58:01AM -0700, Tamas K Lengyel wrote:
> >>> On Thu, Dec 19, 2019 at 2:48 AM Roger Pau Monné  
> >>> wrote:
> 
>  On Wed, Dec 18, 2019 at 11:40:37AM -0800, Tamas K Lengyel wrote:
> > The following series implements VM forking for Intel HVM guests to 
> > allow for
> > the fast creation of identical VMs without the assosciated high startup 
> > costs
> > of booting or restoring the VM from a savefile.
> >
> > JIRA issue: https://xenproject.atlassian.net/browse/XEN-89
> >
> > The main design goal with this series has been to reduce the time of 
> > creating
> > the VM fork as much as possible. To achieve this the VM forking process 
> > is
> > split into two steps:
> >  1) forking the VM on the hypervisor side;
> >  2) starting QEMU to handle the backed for emulated devices.
> >
> > Step 1) involves creating a VM using the new "xl fork-vm" command. The
> > parent VM is expected to remain paused after forks are created from it 
> > (which
> > is different then what process forking normally entails). During this 
> > forking
>  ^ than
> > operation the HVM context and VM settings are copied over to the new 
> > forked VM.
> > This operation is fast and it allows the forked VM to be unpaused and 
> > to be
> > monitored and accessed via VMI. Note however that without its device 
> > model
> > running (depending on what is executing in the VM) it is bound to
> > misbehave/crash when its trying to access devices that would be 
> > emulated by
> > QEMU. We anticipate that for certain use-cases this would be an 
> > acceptable
> > situation, in case for example when fuzzing is performed of code 
> > segments that
> > don't access such devices.
> >
> > Step 2) involves launching QEMU to support the forked VM, which 
> > requires the
> > QEMU Xen savefile to be generated manually from the parent VM. This can 
> > be
> > accomplished simply by connecting to its QMP socket and issuing the
> > "xen-save-devices-state" command as documented by QEMU:
> > https://github.com/qemu/qemu/blob/master/docs/xen-save-devices-state.txt
> > Once the QEMU Xen savefile is generated the new "xl fork-launch-dm" 
> > command is
> > used to launch QEMU and load the specified savefile for it.
> 
>  IMO having two different commands is confusing for the end user, I
>  would rather have something like:
> 
>  xl fork-vm [-d] ...
> 
>  Where '-d' would prevent forking any user-space emulators. I don't
>  thinks there's a need for a separate command to fork the underlying
>  user-space emulators.
> >>>
> >>> Keeping it as two commands allows you to start up the fork and let it
> >>> run immediately and only start up QEMU when you notice it is needed.
> >>> The idea being that you can monitor the kernel and see when it tries
> >>> to do some I/O that would require the QEMU backend. If you combine the
> >>> commands that option goes away.
> >>
> >> I'm not sure I see why, you could still provide a `xl fork-vm [-c]
> >> ...` that would just lunch a QEMU instance. End users using xl have
> >> AFAICT no way to tell whether or when a QEMU is needed or not, and
> >> hence the default behavior should be a fully functional one.
> >>
> >> IMO I think fork-vm without any options should do a complete fork of a
> >> VM, rather than a partial one without a device model clone.
> >
> > I understand your point but implementing that is outside the scope of
> > what we are doing right now. There are a lot more steps involved if
> > you want to create a fully functional VM fork with QEMU, for example
> > you also have to create a separate disk so you don't clobber the
> > parent VM's disk. Also, saving the QEMU device state is currently
> > hard-wired into the save/migration operation, so changing that
> > plumbing in libxl is quite involved. I actually found it way easier to
> > just write a script that connects to the socket and saves it to a
> > target file then going through the pain of adjusting libxl. So while
> > this could be implemented at this time it won't be.
> That's fine to not implement it right now, however the user interface
> should be able to cater it.
>
> In this case, I agree with Roger that it is more intuitive to think that
> fork means a complete fork, not a partial one.
>
> You could impose the user to always pass that option to not clone the
> device model and return an error if it is not there.

Just to be clear, I can add the option to the "fork-vm" command to
load the QEMU state with it, effectively combining the "fork-vm" and
"fork-launch-dm" into one. But I still need 

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail REGR. vs. 
145025

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-amd64-pvgrub  7 xen-boot  fail pass in 145377

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

version targeted for testing:
 xen  3a13ac3ad4d3ef399fe2c85fb09fcb7ab1cdd140
baseline version:
 xen  0cd791c499bdc698d14a24050ec56d60b45732e0

Last test of basis   145025  2019-12-20 13:58:10 Z   10 days
Failing since145058  2019-12-21 07:15:37 Z9 days   23 attempts
Testing same since   

[Xen-devel] [PATCH v4 6/7] Add guide on Communication Best Practice

[Lars Kurth]

From: Lars Kurth 

This guide covers the bulk on Best Practice related to code review
It primarily focusses on code review interactions
It also covers how to deal with Misunderstandings and Cultural
Differences

Changes since v3
* Fixed typo

Changes since v2 (added in v2)
* Fix typos
* Extended "Verbose vs. terse"
* Added "Clarity over Verbosity"
* Broke "Identify the severity of an issue or disagreement" into two chapters
  - "Identify the severity and optionality of review comments" and made
clarifications
  - "Identify the severity of a disagreement"
  - Expanded "Prioritize significant flaws"
* Added "Reviewers: Take account of previous reviewer(s) comments"
* Added prefixes such as "Reviewers:" where appropriate
* Fixed lien wrapping to 80 characters
* Replaced inline links with reference links

Signed-off-by: Lars Kurth 
---
Cc: minios-de...@lists.xenproject.org
Cc: xen-...@lists.xenproject.org
Cc: win-pv-de...@lists.xenproject.org
Cc: mirageos-de...@lists.xenproject.org
Cc: committ...@xenproject.org
---
 communication-practice.md | 504 ++
 1 file changed, 504 insertions(+)
 create mode 100644 communication-practice.md

diff --git a/communication-practice.md b/communication-practice.md
new file mode 100644
index 000..438b73a
--- /dev/null
+++ b/communication-practice.md
@@ -0,0 +1,504 @@
+# Communication Best Practice
+
+This guide provides communication Best Practice that helps you in
+* Using welcoming and inclusive language
+* Keeping discussions technical and actionable
+* Being respectful of differing viewpoints and experiences
+* Being aware of your own and counterpart???s communication style and culture
+* Show empathy towards other community members
+
+## Code reviews for **reviewers** and **patch authors**
+
+Before embarking on a code review, it is important to remember that
+* A poorly executed code review can hurt the contributors feeling, even when a
+  reviewer did not intend to do so. Feeling defensive is a normal reaction to
+  a critique or feedback. A reviewer should be aware of how the pitch, tone,
+  or sentiment of their comments could be interpreted by the contributor. The
+  same applies to responses of an author to the reviewer.
+* When reviewing someone's code, you are ultimately looking for issues. A good
+  code reviewer is able to mentally separate finding issues from articulating
+  code review comments in a constructive and positive manner: depending on your
+  personality this can be **difficult** and you may need to develop a technique
+  that works for you.
+* As software engineers we like to be proud of the solutions we came up with.
+  This can make it easy to take another people???s criticism personally. Always
+  remember that it is the code that is being reviewed, not you as a person.
+* When you receive code review feedback, please be aware that we have reviewers
+  from different backgrounds, communication styles and cultures. Although we
+  all trying to create a productive, welcoming and agile environment, we do
+  not always succeed.
+
+### Express appreciation
+
+As the nature of code review to find bugs and possible issues, it is very easy
+for reviewers to get into a mode of operation where the patch review ends up
+being a list of issues, not mentioning what is right and well done. This can
+lead to the code submitter interpreting your feedback in a negative way.
+
+The opening of a code review provides an opportunity to address this and also
+sets the tone for the rest of the code review. Starting **every** review on a
+positive note, helps set the tone for the rest of the review.
+
+For an initial patch, you can use phrases such as
+> Thanks for the patch
+> Thanks for doing this
+
+For further revisions within a review, phrases such as
+> Thank you for addressing the last set of changes
+
+If you believe the code was good, it is good practice to highlight this by
+using phrases such as
+> Looks good, just a few comments
+> The changes you have made since the last version look good
+
+If you think there were issues too many with the code to use one of the
+phrases, you can still start on a positive note, by for example saying
+> I think this is a good change
+> I think this is a good feature proposal
+
+It is also entirely fine to highlight specific changes as good. The best place
+to do this, is at the top of a patch, as addressing code review comments
+typically requires a contributor to go through the list of things to address
+and an in-lined positive comment is likely to break that workflow.
+
+You should also consider, that if you review a patch of an experienced
+contributor phrases such as *Thanks for the patch* could come across as
+patronizing, while using *Thanks for doing this* is less likely to be
+interpreted as such.
+
+Appreciation should also be expressed by patch authors when asking for
+clarifications to a review or responding to questions. A simple
+> Thank you for your feedback

[Xen-devel] [PATCH v4 4/7] Add Communication Guide

[Lars Kurth]

From: Lars Kurth 

This document is a portal page that lays out our gold standard,
best practices for some common situations and mechanisms to help
resolve issues that can have a negative effect on our community.

Detail is covered in subsequent documents

Changes since v3
- Also changes the TODO in code-of-conduct.md which had been lost
  in v2

Changes since v2 (introduced in v2)
- Make lines break at 80 characters

Signed-off-by: Lars Kurth 
---
Cc: minios-de...@lists.xenproject.org
Cc: xen-...@lists.xenproject.org
Cc: win-pv-de...@lists.xenproject.org
Cc: mirageos-de...@lists.xenproject.org
Cc: committ...@xenproject.org
---
 code-of-conduct.md |  4 +--
 communication-guide.md | 67 ++
 2 files changed, 69 insertions(+), 2 deletions(-)
 create mode 100644 communication-guide.md

diff --git a/code-of-conduct.md b/code-of-conduct.md
index 7c29a4f..a6080cd 100644
--- a/code-of-conduct.md
+++ b/code-of-conduct.md
@@ -14,7 +14,7 @@ personal appearance, race, religion, or sexual identity and 
orientation.
 We believe that a Code of Conduct can help create a harassment-free 
environment,
 but is not sufficient to create a welcoming environment on its own: guidance on
 creating a welcoming environment, how to communicate in an effective and
-friendly way, etc. can be found [here][guidance].
+friendly way, etc. can be found [here][guidance]].
 
 Examples of unacceptable behavior by participants include:
 
@@ -85,7 +85,7 @@ version 1.4, available at
 https://www.contributor-covenant.org/version/1/4/code-of-conduct.html
 
 [homepage]: https://www.contributor-covenant.org
-[guidance]: TODO-INSERT-URL
+[guidance]: communication-guide.md
 
 For answers to common questions about this code of conduct, see
 https://www.contributor-covenant.org/faq
diff --git a/communication-guide.md b/communication-guide.md
new file mode 100644
index 000..153b100
--- /dev/null
+++ b/communication-guide.md
@@ -0,0 +1,67 @@
+# Communication Guide
+
+We believe that our [Code of Conduct](code-of-conduct.md) can help create a
+harassment-free environment, but is not sufficient to create a welcoming
+environment on its own. We can all make mistakes: when we do, we take
+responsibility for them and try to improve.
+
+This document lays out our gold standard, best practices for some common
+situations and mechanisms to help resolve issues that can have a
+negative effect on our community.
+
+## Goal
+
+We want a productive, welcoming and agile community that can welcome new
+ideas in a complex technical field which is able to reflect on and improve how
+we work.
+
+## Communication & Handling Differences in Opinions
+
+Examples of behavior that contributes to creating a positive environment
+include:
+* Use welcoming and inclusive language
+* Keep discussions technical and actionable
+* Be respectful of differing viewpoints and experiences
+* Be aware of your own and counterpart???s communication style and culture
+* Gracefully accept constructive criticism
+* Focus on what is best for the community
+* Show empathy towards other community members
+* Resolve differences in opinion effectively
+
+## Getting Help
+
+When developing code collaboratively, technical discussion and disagreements
+are unavoidable. Our contributors come from different countries and cultures,
+are driven by different goals and take pride in their work and in their point
+of view. This invariably can lead to lengthy and unproductive debate,
+followed by indecision, sometimes this can impact working relationships
+or lead to other issues that can have a negative effect on our community.
+
+To minimize such issue, we provide a 3-stage process
+* Self-help as outlined in this document
+* Ability to ask for an independent opinion or help in private
+* Mediation between parties which disagree. In this case a neutral community
+  member assists the disputing parties resolve the issues or will work with the
+  parties such that they can improve future interactions.
+
+If you need and independent opinion or help, feel free to contact
+mediat...@xenproject.org. The team behind mediation@ is made up of the
+same community members as those listed in the Conduct Team: see
+[Code of Conduct](code-of-conduct.md). In addition, team members are obligated
+to maintain confidentiality with regard discussions that take place. If you
+have concerns about any of the members of the mediation@ alias, you are
+welcome to contact precisely the team member(s) of your choice. In this case,
+please make certain that you highlight the nature of a request by making sure
+that either help or mediation is mentioned in the e-mail subject or body.
+
+## Specific Topics and Best Practice
+
+* [Code Review Guide](code-review-guide.md):
+  Essential reading for code reviewers and contributors
+* [Communication Best Practice](communication-practice.md):
+  This guide covers communication guidelines for code reviewers and authors.
+  It should help you 

[Xen-devel] [PATCH v4 0/7] Code of Conduct + Extra Guides and Best Practices + VOTE

[Lars Kurth]

From: Lars Kurth 

This series proposes a concrete version of the Xen Project
CoC based on v1.4 of the Contributor Covenant. See [1]

Closing the discussion
==
I think we are at the point where we are ready to publish our guidance.
Feedback has been minor since the last version and loose ends were primarily
to do with missing examples.

To close this, I wanted to get votes on the proposal in the usual way.
Technically, only leadership team members of mature projects which are
Hypervisor, XAPI and the Windows PV driver project can vote.

However, in this case I do believe we do want to hear voices of others.

Voting would follow the rules outlined in
https://xenproject.org/developers/governance/#project-decisions

Leadership team members should vote by replying if they are happy with the
substance of the proposal by using the usual terminology
+2 : I am happy with this proposal, and I will argue for it
+1 : I am happy with this proposal, but will not argue for it
0 : I have no opinion
-1 : I am not happy with this proposal, but will not argue against it
-2 : I am not happy with this proposal, and I will argue against it

If there are minor changes (such as typos, etc) we should fix this in due
course. If there are major objections, please highlight here but also
raise it against the specific patch and make clear what the objection is.

More notes on Changes
=
It tries to address all elements in the v2 review, which raised
a number of hard questions, which were mostly addressed in v3.

One of the main outstanding items in v3 were good examples for cover letters
and well structured large patch series which were added in v4.

For convenience of review and in line with other policy documents
I created a git repository at [2]. This series can be found at [3].

I also reformatted the series to 80 characters and replaced
inline style links with reference style links to make it easier
to stick to a character limit.

[1] https://www.contributor-covenant.org/version/1/4/code-of-conduct.md
[2] http://xenbits.xen.org/gitweb/?p=people/larsk/code-of-conduct.git;a=summary
[3] 
http://xenbits.xen.org/gitweb/?p=people/larsk/code-of-conduct.git;a=shortlog;h=refs/heads/CoC-v4

Changes since v3
  * More typo and whitespace fixes

  code-review-guide.md
* Added example under *Workflow from a Reviewer's Perspective* section

Changes since v2
  * Reformatted all text to 80 characters and replaced link style

  code-review-guide.md
  * Extend introduction
  * Add "Code Review Workflow" covering
- "Workflow from a Reviewer's Perspective"
- "Workflow from an Author's Perspective"
- "Problematic Patch Reviews"

  TODO: find suitable examples on how to structure/describe good patch series

  communication-practice.md
  * Fix typos
  * Extended "Verbose vs. terse"
  * Added "Clarity over Verbosity"
  * Broke "Identify the severity of an issue or disagreement" into two chapters
- "Identify the severity and optionality of review comments" and made
  clarifications
- "Identify the severity of a disagreement"
- Expanded "Prioritize significant flaws"
  * Added "Reviewers: Take account of previous reviewer(s) comments"
  * Added prefixes such as "Reviewers:" where appropriate

  resolving-disagreement.md
  * Fix typos
  * Add section: "Issue: Multiple ways to solve a problem"

Changes since v1
* Code of Conduct
  Only whitespace changes

* Added Communication Guide
  Contains values and a process based on advice and mediation in case of issues
  This is the primary portal for

* Added Code Review Guide
  Which is based on [4] with some additions for completeness
  It primarily sets expectations and anything communication related is removed

* Added guide on Communication Best Practice
  Takes the communication section from [4] and expands on it with more examples
  and cases. This is probably where we may need some discussion

* Added document on Resolving Disagreement
  A tiny bit of theory to set the scene
  It covers some common cases of disagreements and how we may approach them
  Again, this probably needs some discussion

Cc: minios-de...@lists.xenproject.org
Cc: xen-...@lists.xenproject.org
Cc: win-pv-de...@lists.xenproject.org
Cc: mirageos-de...@lists.xenproject.org
Cc: committ...@xenproject.org


Lars Kurth (7):
  Import v1.4 of Contributor Covenant CoC
  Xen Project Code of Conduct
  Reformat Xen Project CoC to fit into 80 character limit
  Add Communication Guide
  Add Code Review Guide
  Add guide on Communication Best Practice
  Added Resolving Disagreement

--
2.13.0

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

[Xen-devel] [PATCH v4 5/7] Add Code Review Guide

[Lars Kurth]

From: Lars Kurth 

This document highlights what reviewers such as maintainers and committers look
for when reviewing code. It sets expectations for code authors and provides
a framework for code reviewers.

Changes since v3
* Added example under *Workflow from a Reviewer's Perspective* section
* Fixed typos in text introduced in v2

Changes since v2 (introduced in v2)
* Extend introduction
* Add "Code Review Workflow" covering
  - "Workflow from a Reviewer's Perspective"
  - "Workflow from an Author's Perspective"
  - "Problematic Patch Reviews"
* Wrap to 80 characters
* Replace inline links with reference links to make
  wrapping easier

Signed-off-by: Lars Kurth 
---
Cc: minios-de...@lists.xenproject.org
Cc: xen-...@lists.xenproject.org
Cc: win-pv-de...@lists.xenproject.org
Cc: mirageos-de...@lists.xenproject.org
Cc: committ...@xenproject.org
---
 code-review-guide.md | 313 +++
 1 file changed, 313 insertions(+)
 create mode 100644 code-review-guide.md

diff --git a/code-review-guide.md b/code-review-guide.md
new file mode 100644
index 000..b2c08d2
--- /dev/null
+++ b/code-review-guide.md
@@ -0,0 +1,313 @@
+# Code Review Guide
+
+This document highlights what reviewers such as maintainers and committers look
+for when reviewing your code. It sets expectations for code authors and 
provides
+a framework for code reviewers.
+
+Before we start, it is important to remember that the primary purpose of a
+a code review is to identify any bugs or potential bugs in the code. Most code
+reviews are relatively straight-forward and do not require re-writing the
+submitted code substantially.
+
+The document provides advice on how to structure larger patch series and
+provides  pointers on code author's and reviewer's workflows.
+
+Sometimes it happens that a submitted patch series made wrong assumptions or 
has
+a flawed design or architecture. This can be frustrating for contributors and
+code  reviewers. Note that this document does contain [a section](#problems)
+that provides  suggestions on how to minimize the impact for most stake-holders
+and also on how to avoid such situations.
+
+This document does **not cover** the following topics:
+* [Communication Best Practice][1]
+* [Resolving Disagreement][2]
+* [Patch Submission Workflow][3]
+* [Managing Patch Submission with Git][4]
+
+## What we look for in Code Reviews
+
+When performing a code review, reviewers typically look for the following 
things
+
+### Is the change necessary to accomplish the goals?
+
+* Is it clear what the goals are?
+* Do we need to make a change, or can the goals be met with existing
+  functionality?
+
+### Architecture / Interface
+
+* Is this the best way to solve the problem?
+* Is this the right part of the code to modify?
+* Is this the right level of abstraction?
+* Is the interface general enough? Too general? Forward compatible?
+
+### Functionality
+
+* Does it do what it???s trying to do?
+* Is it doing it in the most ef???cient way?
+* Does it handle all the corner / error cases correctly?
+
+### Maintainability / Robustness
+
+* Is the code clear? Appropriately commented?
+* Does it duplicate another piece of code?
+* Does the code make hidden assumptions?
+* Does it introduce sections which need to be kept **in sync** with other
+  sections?
+* Are there other **traps** someone modifying this code might fall into?
+
+**Note:** Sometimes you will work in areas which have identified 
maintainability
+and/or robustness issues. In such cases, maintainers may ask you to make
+additional changes, such that your submitted code does not make things worse or
+point you to other patches are already being worked on.
+
+### System properties
+
+In some areas of the code, system properties such as
+* Code size
+* Performance
+* Scalability
+* Latency
+* Complexity
+* 
+are also important during code reviews.
+
+### Style
+
+* Comments, carriage returns, **snuggly braces**, 
+* See [CODING_STYLE][5] and [tools/libxl/CODING_STYLE][6]
+* No extraneous whitespace changes
+
+### Documentation and testing
+
+* If there is pre-existing documentation in the tree, such as man pages, design
+  documents, etc. a contributor may be asked to update the documentation
+  alongside the change. Documentation is typically present in the [docs][7]
+  folder.
+* When adding new features that have an impact on the end-user,
+  a contributor should include an update to the [SUPPORT.md][8] file.
+  Typically, more complex features require several patch series before it is
+  ready to be advertised in SUPPORT.md
+* When adding new features, a contributor may be asked to provide tests or
+  ensure that existing tests pass
+
+ Testing for the Xen Project Hypervisor
+
+Tests are typically located in one of the following directories
+* **Unit tests**: [tools/tests][9] or [xen/test][A]
+  Unit testing is hard for a system like Xen and typically requires building a
+  subsystem of your tree. If your 

[Xen-devel] [PATCH v4 1/7] Import v1.4 of Contributor Covenant CoC

[Lars Kurth]

From: Lars Kurth 

Signed-off-by: Lars Kurth 
---
Cc: minios-de...@lists.xenproject.org
Cc: xen-...@lists.xenproject.org
Cc: win-pv-de...@lists.xenproject.org
Cc: mirageos-de...@lists.xenproject.org
Cc: committ...@xenproject.org
---
 code-of-conduct.md | 76 ++
 1 file changed, 76 insertions(+)
 create mode 100644 code-of-conduct.md

diff --git a/code-of-conduct.md b/code-of-conduct.md
new file mode 100644
index 000..81b217c
--- /dev/null
+++ b/code-of-conduct.md
@@ -0,0 +1,76 @@
+# Contributor Covenant Code of Conduct
+
+## Our Pledge
+
+In the interest of fostering an open and welcoming environment, we as
+contributors and maintainers pledge to make participation in our project and
+our community a harassment-free experience for everyone, regardless of age, 
body
+size, disability, ethnicity, sex characteristics, gender identity and 
expression,
+level of experience, education, socio-economic status, nationality, personal
+appearance, race, religion, or sexual identity and orientation.
+
+## Our Standards
+
+Examples of behavior that contributes to creating a positive environment
+include:
+
+* Using welcoming and inclusive language
+* Being respectful of differing viewpoints and experiences
+* Gracefully accepting constructive criticism
+* Focusing on what is best for the community
+* Showing empathy towards other community members
+
+Examples of unacceptable behavior by participants include:
+
+* The use of sexualized language or imagery and unwelcome sexual attention or
+  advances
+* Trolling, insulting/derogatory comments, and personal or political attacks
+* Public or private harassment
+* Publishing others' private information, such as a physical or electronic
+  address, without explicit permission
+* Other conduct which could reasonably be considered inappropriate in a
+  professional setting
+
+## Our Responsibilities
+
+Project maintainers are responsible for clarifying the standards of acceptable
+behavior and are expected to take appropriate and fair corrective action in
+response to any instances of unacceptable behavior.
+
+Project maintainers have the right and responsibility to remove, edit, or
+reject comments, commits, code, wiki edits, issues, and other contributions
+that are not aligned to this Code of Conduct, or to ban temporarily or
+permanently any contributor for other behaviors that they deem inappropriate,
+threatening, offensive, or harmful.
+
+## Scope
+
+This Code of Conduct applies within all project spaces, and it also applies 
when
+an individual is representing the project or its community in public spaces.
+Examples of representing a project or community include using an official
+project e-mail address, posting via an official social media account, or acting
+as an appointed representative at an online or offline event. Representation of
+a project may be further defined and clarified by project maintainers.
+
+## Enforcement
+
+Instances of abusive, harassing, or otherwise unacceptable behavior may be
+reported by contacting the project team at [INSERT EMAIL ADDRESS]. All
+complaints will be reviewed and investigated and will result in a response that
+is deemed necessary and appropriate to the circumstances. The project team is
+obligated to maintain confidentiality with regard to the reporter of an 
incident.
+Further details of specific enforcement policies may be posted separately.
+
+Project maintainers who do not follow or enforce the Code of Conduct in good
+faith may face temporary or permanent repercussions as determined by other
+members of the project's leadership.
+
+## Attribution
+
+This Code of Conduct is adapted from the [Contributor Covenant][homepage], 
version 1.4,
+available at 
https://www.contributor-covenant.org/version/1/4/code-of-conduct.html
+
+[homepage]: https://www.contributor-covenant.org
+
+For answers to common questions about this code of conduct, see
+https://www.contributor-covenant.org/faq
-- 
2.13.0


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

[Xen-devel] [PATCH v4 3/7] Reformat Xen Project CoC to fit into 80 character limit

[Lars Kurth]

From: Lars Kurth 

No content changes

Signed-off-by: Lars Kurth 
---
Cc: minios-de...@lists.xenproject.org
Cc: xen-...@lists.xenproject.org
Cc: win-pv-de...@lists.xenproject.org
Cc: mirageos-de...@lists.xenproject.org
Cc: committ...@xenproject.org
---
 code-of-conduct.md | 62 +-
 1 file changed, 33 insertions(+), 29 deletions(-)

diff --git a/code-of-conduct.md b/code-of-conduct.md
index ee751a7..7c29a4f 100644
--- a/code-of-conduct.md
+++ b/code-of-conduct.md
@@ -5,16 +5,16 @@
 In the interest of fostering an open and welcoming environment, we as
 contributors and maintainers pledge to make participation in our project and
 our community a harassment-free experience for everyone, regardless of age, 
body
-size, disability, ethnicity, sex characteristics, gender identity and 
expression,
-level of experience, education, socio-economic status, nationality, personal
-appearance, race, religion, or sexual identity and orientation.
+size, disability, ethnicity, sex characteristics, gender identity and
+expression, level of experience, education, socio-economic status, nationality,
+personal appearance, race, religion, or sexual identity and orientation.
 
 ## Our Standards
 
 We believe that a Code of Conduct can help create a harassment-free 
environment,
 but is not sufficient to create a welcoming environment on its own: guidance on
-creating a welcoming environment, how to communicate in an effective and 
friendly
-way, etc. can be found [here]: TODO-INSERT-URL.
+creating a welcoming environment, how to communicate in an effective and
+friendly way, etc. can be found [here][guidance].
 
 Examples of unacceptable behavior by participants include:
 
@@ -29,41 +29,43 @@ Examples of unacceptable behavior by participants include:
 
 ## Our Responsibilities
 
-Project leadership team members are responsible for clarifying the standards 
of acceptable
-behavior and are expected to take appropriate and fair corrective action in
-response to any instances of unacceptable behavior.
+Project leadership team members are responsible for clarifying the standards of
+acceptable behavior and are expected to take appropriate and fair corrective
+action in response to any instances of unacceptable behavior.
 
-Project leadership team members have the right and responsibility to remove, 
edit, or
-reject comments, commits, code, wiki edits, issues, and other contributions
-that are not aligned to this Code of Conduct, or to ban temporarily or
-permanently any contributor for other behaviors that they deem inappropriate,
-threatening, offensive, or harmful.
+Project leadership team members have the right and responsibility to remove,
+edit, or reject comments, commits, code, wiki edits, issues, and other
+contributions that are not aligned to this Code of Conduct, or to ban
+temporarily or permanently any contributor for other behaviors that they deem
+inappropriate, threatening, offensive, or harmful.
 
 ## Scope
 
-This Code of Conduct applies within all project spaces of all sub-projects, 
and it also applies when
-an individual is representing the project or its community in public spaces.
-Examples of representing a project or community include using an official
-project e-mail address, posting via an official social media account, or acting
-as an appointed representative at an online or offline event. Representation of
-a project may be further defined and clarified by the project leadership.
+This Code of Conduct applies within all project spaces of all sub-projects,
+and it also applies when an individual is representing the project or its
+community in public spaces. Examples of representing a project or community
+include using an official project e-mail address, posting via an official 
social
+media account, or acting as an appointed representative at an online or offline
+event. Representation of a project may be further defined and clarified by the
+project leadership.
 
 ## What to do if you witness or are subject to unacceptable behavior
 
 Instances of abusive, harassing, or otherwise unacceptable behavior may be
 reported by contacting Conduct Team members at cond...@xenproject.org. All
 complaints will be reviewed and investigated and will result in a response that
-is deemed necessary and appropriate to the circumstances. Conduct Team members 
are
-obligated to maintain confidentiality with regard to the reporter of an 
incident.
-Further details of specific enforcement policies may be posted separately.
+is deemed necessary and appropriate to the circumstances. Conduct Team members
+are obligated to maintain confidentiality with regard to the reporter of an
+incident. Further details of specific enforcement policies may be posted
+separately.
 
 If you have concerns about any of the members of the conduct@ alias,
 you are welcome to contact precisely the Conduct Team member(s) of
 your choice.
 
-Project leadership team members who do not follow or 

[Xen-devel] [PATCH v4 7/7] Added Resolving Disagreement

[Lars Kurth]

From: Lars Kurth 

This guide provides Best Practice on identifying and resolving
common classes of disagreement

Changes since v3
* Fixed broken http link (typo)

Changes since v2 (added in v2)
* Fix typos
* Add section: "Issue: Multiple ways to solve a problem"
* Changed line wrapping to 80 characters
* Replaced inline style links with reference style links

Signed-off-by: Lars Kurth 
--
Cc: minios-de...@lists.xenproject.org
Cc: xen-...@lists.xenproject.org
Cc: win-pv-de...@lists.xenproject.org
Cc: mirageos-de...@lists.xenproject.org
Cc: committ...@xenproject.org
---
 resolving-disagreement.md | 188 ++
 1 file changed, 188 insertions(+)
 create mode 100644 resolving-disagreement.md

diff --git a/resolving-disagreement.md b/resolving-disagreement.md
new file mode 100644
index 000..fb3b134
--- /dev/null
+++ b/resolving-disagreement.md
@@ -0,0 +1,188 @@
+# Resolving Disagreement
+
+This guide provides Best Practice on resolving disagreement, such as
+* Gracefully accept constructive criticism
+* Focus on what is best for the community
+* Resolve differences in opinion effectively
+
+## Theory: Paul Graham's hierarchy of disagreement
+
+Paul Graham proposed a **disagreement hierarchy** in a 2008 essay
+**[How to Disagree][1]**, putting types of arguments into a seven-point
+hierarchy and observing that *moving up the disagreement hierarchy makes people
+less mean, and will make most of them happier*. Graham also suggested that the
+hierarchy can be thought of as a pyramid, as the highest forms of disagreement
+are rarer.
+
+| ![Graham's Hierarchy of Disagreement][2]|
+| *A representation of Graham's hierarchy of disagreement from [Loudacris][3]
+  modified by [Rocket000][4]* |
+
+In the context of the Xen Project we strive to **only use the top half** of the
+hierarchy. **Name-calling** and **Ad hominem** arguments are not acceptable
+within the Xen Project.
+
+## Issue: Scope creep
+
+One thing which occasionally happens during code review is that a code reviewer
+asks or appears to ask the author of a patch to implement additional
+functionalities.
+
+This could take for example the form of
+> Do you think it would be useful for the code to do XXX?
+> I can imagine a user wanting to do YYY (and XXX would enable this)
+
+That potentially adds additional work for the code author, which they may not
+have the time to perform. It is good practice for authors to consider such a
+request in terms of
+* Usefulness to the user
+* Code churn, complexity or impact on other system properties
+* Extra time to implement and report back to the reviewer
+
+If you believe that the impact/cost is too high, report back to the reviewer.
+To resolve this, it is advisable to
+* Report your findings
+* And then check whether this was merely an interesting suggestion, or 
something
+  the reviewer feels more strongly about
+
+In the latter case, there are typically several common outcomes
+* The **author and reviewer agree** that the suggestion should be implemented
+* The **author and reviewer agree** that it may make sense to defer
+  implementation
+* The **author and reviewer agree** that it makes no sense to implement the
+  suggestion
+
+The author of a patch would typically suggest their preferred outcome, for
+example
+> I am not sure it is worth to implement XXX
+> Do you think this could be done as a separate patch in future?
+
+In cases, where no agreement can be found, the best approach would be to get an
+independent opinion from another maintainer or the project's leadership team.
+
+## Issue: [Bikeshedding][5]
+
+Occasionally discussions about unimportant but easy-to-grasp issues can lead to
+prolonged and unproductive discussions. The best way to approach this is to
+try and **anticipate** bikeshedding and highlight it as such upfront. However,
+the format of a code review does not always lend itself well to this approach,
+except for highlighting it in the cover letter of a patch series.
+
+However, typically Bikeshedding issues are fairly easy to recognize in a code
+review, as you will very quickly get different reviewers providing differing
+opinions. In this case it is best for the author or a reviewer to call out the
+potential bikeshedding issue using something like
+
+> Looks we have a bikeshedding issue here
+> I think we should call a quick vote to settle the issue
+
+Our governance provides the mechanisms of [informal votes][6] or
+[lazy voting][7] which lend themselves well to resolve such issues.
+
+## Issue: Small functional issues
+
+The most common area of disagreements which happen in code reviews, are
+differing opinions on whether small functional issues in a patch series have to
+be resolved or not before the code is ready to be submitted. Such disagreements
+are typically caused by different expectations related to the level of
+perfection a patch series needs 

[Xen-devel] [PATCH v4 2/7] Xen Project Code of Conduct

[Lars Kurth]

From: Lars Kurth 

Specific changes to the baseline:
* Replace list of positive behaviors with link to separate process
* Replace maintainers with project leadership
  (except in our pledge where maintainers is more appropriate)
* Add 'of all sub-projects' to clarify scope of CoC
* Rename Enforcement
* Replace "project team" with "Conduct Team members"
* Add e-mail alias
* Add section on contacting individual Conduct Team members
* Add section on Conduct Team members

Signed-off-by: Lars Kurth 
---
Cc: minios-de...@lists.xenproject.org
Cc: xen-...@lists.xenproject.org
Cc: win-pv-de...@lists.xenproject.org
Cc: mirageos-de...@lists.xenproject.org
Cc: committ...@xenproject.org
---
 code-of-conduct.md | 45 -
 1 file changed, 28 insertions(+), 17 deletions(-)

diff --git a/code-of-conduct.md b/code-of-conduct.md
index 81b217c..ee751a7 100644
--- a/code-of-conduct.md
+++ b/code-of-conduct.md
@@ -1,4 +1,4 @@
-# Contributor Covenant Code of Conduct
+# Xen Project Code of Conduct
 
 ## Our Pledge
 
@@ -11,14 +11,10 @@ appearance, race, religion, or sexual identity and 
orientation.
 
 ## Our Standards
 
-Examples of behavior that contributes to creating a positive environment
-include:
-
-* Using welcoming and inclusive language
-* Being respectful of differing viewpoints and experiences
-* Gracefully accepting constructive criticism
-* Focusing on what is best for the community
-* Showing empathy towards other community members
+We believe that a Code of Conduct can help create a harassment-free 
environment,
+but is not sufficient to create a welcoming environment on its own: guidance on
+creating a welcoming environment, how to communicate in an effective and 
friendly
+way, etc. can be found [here]: TODO-INSERT-URL.
 
 Examples of unacceptable behavior by participants include:
 
@@ -33,11 +29,11 @@ Examples of unacceptable behavior by participants include:
 
 ## Our Responsibilities
 
-Project maintainers are responsible for clarifying the standards of acceptable
+Project leadership team members are responsible for clarifying the standards 
of acceptable
 behavior and are expected to take appropriate and fair corrective action in
 response to any instances of unacceptable behavior.
 
-Project maintainers have the right and responsibility to remove, edit, or
+Project leadership team members have the right and responsibility to remove, 
edit, or
 reject comments, commits, code, wiki edits, issues, and other contributions
 that are not aligned to this Code of Conduct, or to ban temporarily or
 permanently any contributor for other behaviors that they deem inappropriate,
@@ -45,26 +41,40 @@ threatening, offensive, or harmful.
 
 ## Scope
 
-This Code of Conduct applies within all project spaces, and it also applies 
when
+This Code of Conduct applies within all project spaces of all sub-projects, 
and it also applies when
 an individual is representing the project or its community in public spaces.
 Examples of representing a project or community include using an official
 project e-mail address, posting via an official social media account, or acting
 as an appointed representative at an online or offline event. Representation of
-a project may be further defined and clarified by project maintainers.
+a project may be further defined and clarified by the project leadership.
 
-## Enforcement
+## What to do if you witness or are subject to unacceptable behavior
 
 Instances of abusive, harassing, or otherwise unacceptable behavior may be
-reported by contacting the project team at [INSERT EMAIL ADDRESS]. All
+reported by contacting Conduct Team members at cond...@xenproject.org. All
 complaints will be reviewed and investigated and will result in a response that
-is deemed necessary and appropriate to the circumstances. The project team is
+is deemed necessary and appropriate to the circumstances. Conduct Team members 
are
 obligated to maintain confidentiality with regard to the reporter of an 
incident.
 Further details of specific enforcement policies may be posted separately.
 
-Project maintainers who do not follow or enforce the Code of Conduct in good
+If you have concerns about any of the members of the conduct@ alias,
+you are welcome to contact precisely the Conduct Team member(s) of
+your choice.
+
+Project leadership team members who do not follow or enforce the Code of 
Conduct in good
 faith may face temporary or permanent repercussions as determined by other
 members of the project's leadership.
 
+## Conduct Team members
+Conduct Team members are project leadership team members from any
+sub-project. The current list of Conduct Team members is:
+* Lars Kurth 
+* George Dunlap 
+* Ian Jackson 
+
+Conduct Team members are changed by proposing a change to this document,
+posted on all sub-project lists, followed by a formal global vote as outlined 
[here]: https://xenproject.org/developers/governance/#project-decisions
+
 ## Attribution
 
 This Code of 

Re: [Xen-devel] [PATCH v2 00/20] VM forking

[Julien Grall]

Hi Tamas,

On 30/12/2019 18:15, Tamas K Lengyel wrote:

On Mon, Dec 30, 2019 at 10:59 AM Roger Pau Monné  wrote:


On Thu, Dec 19, 2019 at 08:58:01AM -0700, Tamas K Lengyel wrote:

On Thu, Dec 19, 2019 at 2:48 AM Roger Pau Monné  wrote:


On Wed, Dec 18, 2019 at 11:40:37AM -0800, Tamas K Lengyel wrote:

The following series implements VM forking for Intel HVM guests to allow for
the fast creation of identical VMs without the assosciated high startup costs
of booting or restoring the VM from a savefile.

JIRA issue: https://xenproject.atlassian.net/browse/XEN-89

The main design goal with this series has been to reduce the time of creating
the VM fork as much as possible. To achieve this the VM forking process is
split into two steps:
 1) forking the VM on the hypervisor side;
 2) starting QEMU to handle the backed for emulated devices.

Step 1) involves creating a VM using the new "xl fork-vm" command. The
parent VM is expected to remain paused after forks are created from it (which
is different then what process forking normally entails). During this forking

^ than

operation the HVM context and VM settings are copied over to the new forked VM.
This operation is fast and it allows the forked VM to be unpaused and to be
monitored and accessed via VMI. Note however that without its device model
running (depending on what is executing in the VM) it is bound to
misbehave/crash when its trying to access devices that would be emulated by
QEMU. We anticipate that for certain use-cases this would be an acceptable
situation, in case for example when fuzzing is performed of code segments that
don't access such devices.

Step 2) involves launching QEMU to support the forked VM, which requires the
QEMU Xen savefile to be generated manually from the parent VM. This can be
accomplished simply by connecting to its QMP socket and issuing the
"xen-save-devices-state" command as documented by QEMU:
https://github.com/qemu/qemu/blob/master/docs/xen-save-devices-state.txt
Once the QEMU Xen savefile is generated the new "xl fork-launch-dm" command is
used to launch QEMU and load the specified savefile for it.


IMO having two different commands is confusing for the end user, I
would rather have something like:

xl fork-vm [-d] ...

Where '-d' would prevent forking any user-space emulators. I don't
thinks there's a need for a separate command to fork the underlying
user-space emulators.


Keeping it as two commands allows you to start up the fork and let it
run immediately and only start up QEMU when you notice it is needed.
The idea being that you can monitor the kernel and see when it tries
to do some I/O that would require the QEMU backend. If you combine the
commands that option goes away.


I'm not sure I see why, you could still provide a `xl fork-vm [-c]
...` that would just lunch a QEMU instance. End users using xl have
AFAICT no way to tell whether or when a QEMU is needed or not, and
hence the default behavior should be a fully functional one.

IMO I think fork-vm without any options should do a complete fork of a
VM, rather than a partial one without a device model clone.


I understand your point but implementing that is outside the scope of
what we are doing right now. There are a lot more steps involved if
you want to create a fully functional VM fork with QEMU, for example
you also have to create a separate disk so you don't clobber the
parent VM's disk. Also, saving the QEMU device state is currently
hard-wired into the save/migration operation, so changing that
plumbing in libxl is quite involved. I actually found it way easier to
just write a script that connects to the socket and saves it to a
target file then going through the pain of adjusting libxl. So while
this could be implemented at this time it won't be.
That's fine to not implement it right now, however the user interface 
should be able to cater it.


In this case, I agree with Roger that it is more intuitive to think that 
fork means a complete fork, not a partial one.


You could impose the user to always pass that option to not clone the 
device model and return an error if it is not there.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2 00/20] VM forking

[Tamas K Lengyel]

On Mon, Dec 30, 2019 at 10:59 AM Roger Pau Monné  wrote:
>
> On Thu, Dec 19, 2019 at 08:58:01AM -0700, Tamas K Lengyel wrote:
> > On Thu, Dec 19, 2019 at 2:48 AM Roger Pau Monné  
> > wrote:
> > >
> > > On Wed, Dec 18, 2019 at 11:40:37AM -0800, Tamas K Lengyel wrote:
> > > > The following series implements VM forking for Intel HVM guests to 
> > > > allow for
> > > > the fast creation of identical VMs without the assosciated high startup 
> > > > costs
> > > > of booting or restoring the VM from a savefile.
> > > >
> > > > JIRA issue: https://xenproject.atlassian.net/browse/XEN-89
> > > >
> > > > The main design goal with this series has been to reduce the time of 
> > > > creating
> > > > the VM fork as much as possible. To achieve this the VM forking process 
> > > > is
> > > > split into two steps:
> > > > 1) forking the VM on the hypervisor side;
> > > > 2) starting QEMU to handle the backed for emulated devices.
> > > >
> > > > Step 1) involves creating a VM using the new "xl fork-vm" command. The
> > > > parent VM is expected to remain paused after forks are created from it 
> > > > (which
> > > > is different then what process forking normally entails). During this 
> > > > forking
> > >^ than
> > > > operation the HVM context and VM settings are copied over to the new 
> > > > forked VM.
> > > > This operation is fast and it allows the forked VM to be unpaused and 
> > > > to be
> > > > monitored and accessed via VMI. Note however that without its device 
> > > > model
> > > > running (depending on what is executing in the VM) it is bound to
> > > > misbehave/crash when its trying to access devices that would be 
> > > > emulated by
> > > > QEMU. We anticipate that for certain use-cases this would be an 
> > > > acceptable
> > > > situation, in case for example when fuzzing is performed of code 
> > > > segments that
> > > > don't access such devices.
> > > >
> > > > Step 2) involves launching QEMU to support the forked VM, which 
> > > > requires the
> > > > QEMU Xen savefile to be generated manually from the parent VM. This can 
> > > > be
> > > > accomplished simply by connecting to its QMP socket and issuing the
> > > > "xen-save-devices-state" command as documented by QEMU:
> > > > https://github.com/qemu/qemu/blob/master/docs/xen-save-devices-state.txt
> > > > Once the QEMU Xen savefile is generated the new "xl fork-launch-dm" 
> > > > command is
> > > > used to launch QEMU and load the specified savefile for it.
> > >
> > > IMO having two different commands is confusing for the end user, I
> > > would rather have something like:
> > >
> > > xl fork-vm [-d] ...
> > >
> > > Where '-d' would prevent forking any user-space emulators. I don't
> > > thinks there's a need for a separate command to fork the underlying
> > > user-space emulators.
> >
> > Keeping it as two commands allows you to start up the fork and let it
> > run immediately and only start up QEMU when you notice it is needed.
> > The idea being that you can monitor the kernel and see when it tries
> > to do some I/O that would require the QEMU backend. If you combine the
> > commands that option goes away.
>
> I'm not sure I see why, you could still provide a `xl fork-vm [-c]
> ...` that would just lunch a QEMU instance. End users using xl have
> AFAICT no way to tell whether or when a QEMU is needed or not, and
> hence the default behavior should be a fully functional one.
>
> IMO I think fork-vm without any options should do a complete fork of a
> VM, rather than a partial one without a device model clone.

I understand your point but implementing that is outside the scope of
what we are doing right now. There are a lot more steps involved if
you want to create a fully functional VM fork with QEMU, for example
you also have to create a separate disk so you don't clobber the
parent VM's disk. Also, saving the QEMU device state is currently
hard-wired into the save/migration operation, so changing that
plumbing in libxl is quite involved. I actually found it way easier to
just write a script that connects to the socket and saves it to a
target file then going through the pain of adjusting libxl. So while
this could be implemented at this time it won't be.

Tamas

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

Re: [Xen-devel] [PATCH v2 00/20] VM forking

[Roger Pau Monné]

On Thu, Dec 19, 2019 at 08:58:01AM -0700, Tamas K Lengyel wrote:
> On Thu, Dec 19, 2019 at 2:48 AM Roger Pau Monné  wrote:
> >
> > On Wed, Dec 18, 2019 at 11:40:37AM -0800, Tamas K Lengyel wrote:
> > > The following series implements VM forking for Intel HVM guests to allow 
> > > for
> > > the fast creation of identical VMs without the assosciated high startup 
> > > costs
> > > of booting or restoring the VM from a savefile.
> > >
> > > JIRA issue: https://xenproject.atlassian.net/browse/XEN-89
> > >
> > > The main design goal with this series has been to reduce the time of 
> > > creating
> > > the VM fork as much as possible. To achieve this the VM forking process is
> > > split into two steps:
> > > 1) forking the VM on the hypervisor side;
> > > 2) starting QEMU to handle the backed for emulated devices.
> > >
> > > Step 1) involves creating a VM using the new "xl fork-vm" command. The
> > > parent VM is expected to remain paused after forks are created from it 
> > > (which
> > > is different then what process forking normally entails). During this 
> > > forking
> >^ than
> > > operation the HVM context and VM settings are copied over to the new 
> > > forked VM.
> > > This operation is fast and it allows the forked VM to be unpaused and to 
> > > be
> > > monitored and accessed via VMI. Note however that without its device model
> > > running (depending on what is executing in the VM) it is bound to
> > > misbehave/crash when its trying to access devices that would be emulated 
> > > by
> > > QEMU. We anticipate that for certain use-cases this would be an acceptable
> > > situation, in case for example when fuzzing is performed of code segments 
> > > that
> > > don't access such devices.
> > >
> > > Step 2) involves launching QEMU to support the forked VM, which requires 
> > > the
> > > QEMU Xen savefile to be generated manually from the parent VM. This can be
> > > accomplished simply by connecting to its QMP socket and issuing the
> > > "xen-save-devices-state" command as documented by QEMU:
> > > https://github.com/qemu/qemu/blob/master/docs/xen-save-devices-state.txt
> > > Once the QEMU Xen savefile is generated the new "xl fork-launch-dm" 
> > > command is
> > > used to launch QEMU and load the specified savefile for it.
> >
> > IMO having two different commands is confusing for the end user, I
> > would rather have something like:
> >
> > xl fork-vm [-d] ...
> >
> > Where '-d' would prevent forking any user-space emulators. I don't
> > thinks there's a need for a separate command to fork the underlying
> > user-space emulators.
> 
> Keeping it as two commands allows you to start up the fork and let it
> run immediately and only start up QEMU when you notice it is needed.
> The idea being that you can monitor the kernel and see when it tries
> to do some I/O that would require the QEMU backend. If you combine the
> commands that option goes away.

I'm not sure I see why, you could still provide a `xl fork-vm [-c]
...` that would just lunch a QEMU instance. End users using xl have
AFAICT no way to tell whether or when a QEMU is needed or not, and
hence the default behavior should be a fully functional one.

IMO I think fork-vm without any options should do a complete fork of a
VM, rather than a partial one without a device model clone.

> Also, QEMU itself isn't getting forked
> right now, we just start a new QEMU process with the saved-state
> getting loaded into it. I looked into implementing a QEMU fork command
> but it turns out that for the vast majority of our use-cases QEMU
> isn't needed at all, so developing that addition was tabled.

Starting a new process with the saved-state looks fine to me, it can
always be improved afterwards if there's a need for it.

Thanks, Roger.

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

[Xen-devel] Discovering L0 hypervisor features from L2

[Wei Liu]

Hi all

One of the things required to make a Xen system run well on Hyper-V is
to make Dom0 use as many paravirtualised interfaces as possible.

For starters, we would really want Dom0 Linux kernel to use Hyper-V's PV
drivers. But the core VMBus driver is gated by cpuid leaves. If cpuid
returns Xen's signature, VMBus driver won't be loaded.

We will want to different method other than cpuid leaves to communicate
to Linux "this Xen hypervisor is running on top of Hyper-V, thus it is
okay to use Hyper-V drivers". We may also want to be able to indicate
what features are available directly from L0.

There are several ways of doing this:

1. Use a hypervisor specific cpuid leaf.
2. Use an MSR reserved for software use.
3. Use a hypercall.

I generally prefer going with either 1 or 2.

Last but not least, we should make the discovery mechanism generic for
any L0 hypervisor, such that we can easily add support for running Xen
in L0 in the future.

Thoughts, comments and preferences?

Thanks,
Wei.

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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-freebsd10-i386 14 guest-saverestore  fail REGR. vs. 144861
 test-amd64-i386-freebsd10-amd64 14 guest-saverestore fail REGR. vs. 144861
 test-amd64-i386-xl-qemuu-debianhvm-amd64 13 guest-saverestore fail REGR. vs. 
144861
 test-amd64-amd64-xl-qemuu-ovmf-amd64 13 guest-saverestore fail REGR. vs. 144861
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 guest-saverestore fail 
REGR. vs. 144861
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 13 guest-saverestore fail 
REGR. vs. 144861
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 13 guest-saverestore fail REGR. vs. 
144861
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 guest-saverestore fail 
REGR. vs. 144861
 test-amd64-amd64-xl-qemuu-win7-amd64 13 guest-saverestore fail REGR. vs. 144861
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 13 guest-saverestore fail REGR. 
vs. 144861
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 13 guest-saverestore fail REGR. 
vs. 144861
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 13 guest-saverestore fail 
REGR. vs. 144861
 test-amd64-i386-xl-qemuu-ovmf-amd64 13 guest-saverestore fail REGR. vs. 144861
 test-amd64-i386-xl-qemuu-win7-amd64 13 guest-saverestore fail REGR. vs. 144861
 test-amd64-amd64-xl-qemuu-ws16-amd64 13 guest-saverestore fail REGR. vs. 144861
 test-amd64-i386-xl-qemuu-ws16-amd64 13 guest-saverestore fail REGR. vs. 144861

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-rtds15 guest-saverestore fail in 145206 pass in 145388
 test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail pass in 
145046
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeat  fail pass in 145206

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds 17 guest-start.2  fail in 145206 REGR. vs. 144861

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

[Xen-devel] Proxying Hyper-V hypercalls from L2 to L0

[Wei Liu]

Hi all

As much as I try to avoid writing code to proxy Hyper-V hypercalls, it
seems unavoidable for PV guests, because Hyper-V requires hypercalls
to be issued with CPL=0.

This means for PV Dom0 I will need to add code in Xen to support
Hyper-V's ABIs, along with appropriate validations.

How much do you care about running a PV Dom0 in this Xen on Hyper-V
setup? I personally would certainly go full on PVH if possible. :-)

Thanks,
Wei.

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

[Xen-devel] [PATCH v3 18/18] xen/tools: VM forking toolstack side

[Tamas K Lengyel]

Add necessary bits to implement "xl fork-vm", "xl fork-launch-dm" and
"xl fork-reset" commands. The process is split in two to allow tools needing
access to the new VM as fast as possible after it was forked. It is expected
that under certain use-cases the second command that launches QEMU will be
skipped entirely.

Signed-off-by: Tamas K Lengyel 
---
 tools/libxc/include/xenctrl.h |  13 ++
 tools/libxc/xc_memshr.c   |  22 
 tools/libxl/libxl.h   |   7 +
 tools/libxl/libxl_create.c| 237 +++---
 tools/libxl/libxl_dm.c|   2 +-
 tools/libxl/libxl_dom.c   |  83 
 tools/libxl/libxl_internal.h  |   1 +
 tools/libxl/libxl_types.idl   |   1 +
 tools/xl/xl.h |   5 +
 tools/xl/xl_cmdtable.c|  22 
 tools/xl/xl_saverestore.c |  96 ++
 tools/xl/xl_vmcontrol.c   |   8 ++
 12 files changed, 393 insertions(+), 104 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 75f191ae3a..ffb0bb9a42 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2221,6 +2221,19 @@ int xc_memshr_range_share(xc_interface *xch,
   uint64_t first_gfn,
   uint64_t last_gfn);
 
+int xc_memshr_fork(xc_interface *xch,
+   uint32_t source_domain,
+   uint32_t client_domain);
+
+/*
+ * Note: this function is only intended to be used on short-lived forks that
+ * haven't yet aquired a lot of memory. In case the fork has a lot of memory
+ * it is likely more performant to create a new fork with xc_memshr_fork.
+ *
+ * With VMs that have a lot of memory this call may block for a long time.
+ */
+int xc_memshr_fork_reset(xc_interface *xch, uint32_t forked_domain);
+
 /* Debug calls: return the number of pages referencing the shared frame backing
  * the input argument. Should be one or greater.
  *
diff --git a/tools/libxc/xc_memshr.c b/tools/libxc/xc_memshr.c
index 97e2e6a8d9..d0e4ee225b 100644
--- a/tools/libxc/xc_memshr.c
+++ b/tools/libxc/xc_memshr.c
@@ -239,6 +239,28 @@ int xc_memshr_debug_gref(xc_interface *xch,
 return xc_memshr_memop(xch, domid, );
 }
 
+int xc_memshr_fork(xc_interface *xch, uint32_t pdomid, uint32_t domid)
+{
+xen_mem_sharing_op_t mso;
+
+memset(, 0, sizeof(mso));
+
+mso.op = XENMEM_sharing_op_fork;
+mso.u.fork.parent_domain = pdomid;
+
+return xc_memshr_memop(xch, domid, );
+}
+
+int xc_memshr_fork_reset(xc_interface *xch, uint32_t domid)
+{
+xen_mem_sharing_op_t mso;
+
+memset(, 0, sizeof(mso));
+mso.op = XENMEM_sharing_op_fork_reset;
+
+return xc_memshr_memop(xch, domid, );
+}
+
 int xc_memshr_audit(xc_interface *xch)
 {
 xen_mem_sharing_op_t mso;
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 54abb9db1f..75cb070587 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1536,6 +1536,13 @@ int libxl_domain_create_new(libxl_ctx *ctx, 
libxl_domain_config *d_config,
 const libxl_asyncop_how *ao_how,
 const libxl_asyncprogress_how *aop_console_how)
 LIBXL_EXTERNAL_CALLERS_ONLY;
+int libxl_domain_fork_vm(libxl_ctx *ctx, uint32_t pdomid, uint32_t *domid)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+int libxl_domain_fork_launch_dm(libxl_ctx *ctx, libxl_domain_config *d_config,
+uint32_t domid,
+const libxl_asyncprogress_how *aop_console_how)
+LIBXL_EXTERNAL_CALLERS_ONLY;
+int libxl_domain_fork_reset(libxl_ctx *ctx, uint32_t domid);
 int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config,
 uint32_t *domid, int restore_fd,
 int send_back_fd,
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 32d45dcef0..e0d219596c 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -536,12 +536,12 @@ out:
 return ret;
 }
 
-int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
-   libxl__domain_build_state *state,
-   uint32_t *domid)
+static int libxl__domain_make_xs_entries(libxl__gc *gc, libxl_domain_config 
*d_config,
+ libxl__domain_build_state *state,
+ uint32_t domid)
 {
 libxl_ctx *ctx = libxl__gc_owner(gc);
-int ret, rc, nb_vm;
+int rc, nb_vm;
 const char *dom_type;
 char *uuid_string;
 char *dom_path, *vm_path, *libxl_path;
@@ -553,7 +553,6 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config 
*d_config,
 
 /* convenience aliases */
 libxl_domain_create_info *info = _config->c_info;
-libxl_domain_build_info *b_info = _config->b_info;
 
 uuid_string = libxl__uuid2string(gc, info->uuid);
 if 

[Xen-devel] [PATCH v3 17/18] x86/mem_sharing: reset a fork

[Tamas K Lengyel]

Implement hypercall that allows a fork to shed all memory that got allocated
for it during its execution and re-load its vCPU context from the parent VM.
This allows the forked VM to reset into the same state the parent VM is in a
faster way then creating a new fork would be. Measurements show about a 2x
speedup during normal fuzzing operations. Performance may vary depending how
much memory got allocated for the forked VM. If it has been completely
deduplicated from the parent VM then creating a new fork would likely be more
performant.

Signed-off-by: Tamas K Lengyel 
---
v3: use page_list_for_each_safe so we don't have to loop twice
add comments explaining the lack of hypercall continuation
only remove pages doing reset that are of sharable type
---
 xen/arch/x86/mm/mem_sharing.c | 80 +++
 xen/include/public/memory.h   |  1 +
 2 files changed, 81 insertions(+)

diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index d544801681..f16570b01b 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -1607,6 +1607,63 @@ static int mem_sharing_fork(struct domain *d, struct 
domain *cd)
 return 0;
 }
 
+/*
+ * The fork reset operation is intended to be used on short-lived forks only.
+ * There is no hypercall continuation operation implemented for this reason.
+ * For forks that obtain a larger memory footprint it is likely going to be
+ * more performant to create a new fork instead of resetting an existing one.
+ *
+ * TODO: In case this hypercall would become useful on forks with larger memory
+ * footprints the hypercall continuation should be implemented.
+ */
+static int mem_sharing_fork_reset(struct domain *d, struct domain *cd)
+{
+int rc;
+struct p2m_domain* p2m = p2m_get_hostp2m(cd);
+struct page_info *page, *tmp;
+
+if ( !d->controller_pause_count &&
+ (rc = domain_pause_by_systemcontroller(d)) )
+return rc;
+
+page_list_for_each_safe(page, tmp, >page_list)
+{
+p2m_type_t p2mt;
+p2m_access_t p2ma;
+gfn_t gfn;
+mfn_t mfn = page_to_mfn(page);
+
+if ( !mfn_valid(mfn) )
+continue;
+
+gfn = mfn_to_gfn(cd, mfn);
+mfn = __get_gfn_type_access(p2m, gfn_x(gfn), , ,
+0, NULL, false);
+
+/* only remove pages that can be re-populated using sharing */
+if ( !p2m_is_sharable(p2mt) )
+continue;
+
+/* take an extra reference */
+if ( !get_page(page, cd) )
+continue;
+
+rc = p2m->set_entry(p2m, gfn, INVALID_MFN, PAGE_ORDER_4K,
+p2m_invalid, p2m_access_rwx, -1);
+ASSERT(!rc);
+
+put_page_alloc_ref(page);
+put_page(page);
+}
+
+if ( (rc = hvm_copy_context_and_params(d, cd)) )
+return rc;
+
+fork_tsc(d, cd);
+
+return 0;
+}
+
 int mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
 {
 int rc;
@@ -1909,6 +1966,29 @@ int 
mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
 break;
 }
 
+case XENMEM_sharing_op_fork_reset:
+{
+struct domain *pd;
+
+rc = -EINVAL;
+if ( mso.u.fork._pad[0] || mso.u.fork._pad[1] ||
+ mso.u.fork._pad[2] )
+goto out;
+
+rc = -ENOSYS;
+if ( !d->parent )
+goto out;
+
+rc = rcu_lock_live_remote_domain_by_id(d->parent->domain_id, );
+if ( rc )
+goto out;
+
+rc = mem_sharing_fork_reset(pd, d);
+
+rcu_unlock_domain(pd);
+break;
+}
+
 default:
 rc = -ENOSYS;
 break;
diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index 90a3f4498e..e3d063e22e 100644
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -483,6 +483,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_mem_access_op_t);
 #define XENMEM_sharing_op_audit 7
 #define XENMEM_sharing_op_range_share   8
 #define XENMEM_sharing_op_fork  9
+#define XENMEM_sharing_op_fork_reset10
 
 #define XENMEM_SHARING_OP_S_HANDLE_INVALID  (-10)
 #define XENMEM_SHARING_OP_C_HANDLE_INVALID  (-9)
-- 
2.20.1


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

[Xen-devel] [PATCH v3 13/18] x86/mem_sharing: Skip xen heap pages in memshr nominate

[Tamas K Lengyel]

Trying to share these would fail anyway, better to skip them early.

Signed-off-by: Tamas K Lengyel 
---
 xen/arch/x86/mm/mem_sharing.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index b8a9228ecf..baa3e35ded 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -852,6 +852,11 @@ static int nominate_page(struct domain *d, gfn_t gfn,
 if ( !p2m_is_sharable(p2mt) )
 goto out;
 
+/* Skip xen heap pages */
+page = mfn_to_page(mfn);
+if ( !page || is_xen_heap_page(page) )
+goto out;
+
 /* Check if there are mem_access/remapped altp2m entries for this page */
 if ( altp2m_active(d) )
 {
@@ -882,7 +887,6 @@ static int nominate_page(struct domain *d, gfn_t gfn,
 }
 
 /* Try to convert the mfn to the sharable type */
-page = mfn_to_page(mfn);
 ret = page_make_sharable(d, page, expected_refcnt);
 if ( ret )
 goto out;
-- 
2.20.1


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

[Xen-devel] [PATCH v3 06/18] x86/mem_sharing: define mem_sharing_domain to hold some scattered variables

[Tamas K Lengyel]

Create struct mem_sharing_domain under hvm_domain and move mem sharing
variables into it from p2m_domain and hvm_domain.

Expose the mem_sharing_enabled macro to be used consistently across Xen.

Remove some duplicate calls to mem_sharing_enabled in mem_sharing.c

Signed-off-by: Tamas K Lengyel 
---
 xen/arch/x86/mm/mem_sharing.c | 10 --
 xen/drivers/passthrough/pci.c |  3 +--
 xen/include/asm-x86/hvm/domain.h  |  6 +-
 xen/include/asm-x86/mem_sharing.h | 16 
 xen/include/asm-x86/p2m.h |  4 
 5 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index f6187403a0..3aa61c30e6 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -197,9 +197,6 @@ static shr_handle_t get_next_handle(void)
 return x + 1;
 }
 
-#define mem_sharing_enabled(d) \
-(is_hvm_domain(d) && (d)->arch.hvm.mem_sharing_enabled)
-
 static atomic_t nr_saved_mfns   = ATOMIC_INIT(0);
 static atomic_t nr_shared_mfns  = ATOMIC_INIT(0);
 
@@ -1309,6 +1306,7 @@ int __mem_sharing_unshare_page(struct domain *d,
 int relinquish_shared_pages(struct domain *d)
 {
 int rc = 0;
+struct mem_sharing_domain *msd = >arch.hvm.mem_sharing;
 struct p2m_domain *p2m = p2m_get_hostp2m(d);
 unsigned long gfn, count = 0;
 
@@ -1316,7 +1314,7 @@ int relinquish_shared_pages(struct domain *d)
 return 0;
 
 p2m_lock(p2m);
-for ( gfn = p2m->next_shared_gfn_to_relinquish;
+for ( gfn = msd->next_shared_gfn_to_relinquish;
   gfn <= p2m->max_mapped_pfn; gfn++ )
 {
 p2m_access_t a;
@@ -1351,7 +1349,7 @@ int relinquish_shared_pages(struct domain *d)
 {
 if ( hypercall_preempt_check() )
 {
-p2m->next_shared_gfn_to_relinquish = gfn + 1;
+msd->next_shared_gfn_to_relinquish = gfn + 1;
 rc = -ERESTART;
 break;
 }
@@ -1437,7 +1435,7 @@ int 
mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
 
 /* Only HAP is supported */
 rc = -ENODEV;
-if ( !hap_enabled(d) || !d->arch.hvm.mem_sharing_enabled )
+if ( !mem_sharing_enabled(d) )
 goto out;
 
 switch ( mso.op )
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index c07a63981a..65d1d457ff 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1498,8 +1498,7 @@ static int assign_device(struct domain *d, u16 seg, u8 
bus, u8 devfn, u32 flag)
 /* Prevent device assign if mem paging or mem sharing have been 
  * enabled for this domain */
 if ( d != dom_io &&
- unlikely((is_hvm_domain(d) &&
-   d->arch.hvm.mem_sharing_enabled) ||
+ unlikely(mem_sharing_enabled(d) ||
   vm_event_check_ring(d->vm_event_paging) ||
   p2m_get_hostp2m(d)->global_logdirty) )
 return -EXDEV;
diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
index bcc5621797..8f70ba2b1a 100644
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -156,7 +157,6 @@ struct hvm_domain {
 
 struct viridian_domain *viridian;
 
-bool_t mem_sharing_enabled;
 bool_t qemu_mapcache_invalidate;
 bool_t is_s3_suspended;
 
@@ -192,6 +192,10 @@ struct hvm_domain {
 struct vmx_domain vmx;
 struct svm_domain svm;
 };
+
+#ifdef CONFIG_MEM_SHARING
+struct mem_sharing_domain mem_sharing;
+#endif
 };
 
 #endif /* __ASM_X86_HVM_DOMAIN_H__ */
diff --git a/xen/include/asm-x86/mem_sharing.h 
b/xen/include/asm-x86/mem_sharing.h
index cf7848709f..13114b6346 100644
--- a/xen/include/asm-x86/mem_sharing.h
+++ b/xen/include/asm-x86/mem_sharing.h
@@ -26,6 +26,20 @@
 
 #ifdef CONFIG_MEM_SHARING
 
+struct mem_sharing_domain
+{
+bool enabled;
+
+/*
+ * When releasing shared gfn's in a preemptible manner, recall where
+ * to resume the search.
+ */
+unsigned long next_shared_gfn_to_relinquish;
+};
+
+#define mem_sharing_enabled(d) \
+(hap_enabled(d) && (d)->arch.hvm.mem_sharing.enabled)
+
 /* Auditing of memory sharing code? */
 #ifndef NDEBUG
 #define MEM_SHARING_AUDIT 1
@@ -104,6 +118,8 @@ int relinquish_shared_pages(struct domain *d);
 
 #else
 
+#define mem_sharing_enabled(d) false
+
 static inline unsigned int mem_sharing_get_nr_saved_mfns(void)
 {
 return 0;
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 7399c4a897..8defa90306 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -305,10 +305,6 @@ struct p2m_domain {
 unsigned long min_remapped_gfn;
 unsigned long max_remapped_gfn;
 
-/* When releasing shared gfn's in a preemptible manner, recall where
- * to resume the search */
-

[Xen-devel] [PATCH v3 07/18] x86/mem_sharing: Use INVALID_MFN and p2m_is_shared in relinquish_shared_pages

[Tamas K Lengyel]

While using _mfn(0) is of no consequence during teardown, INVALID_MFN is the
correct value that should be used.

Signed-off-by: Tamas K Lengyel 
---
 xen/arch/x86/mm/mem_sharing.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index 3aa61c30e6..95e75ff298 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -1326,7 +1326,7 @@ int relinquish_shared_pages(struct domain *d)
 break;
 
 mfn = p2m->get_entry(p2m, _gfn(gfn), , , 0, NULL, NULL);
-if ( mfn_valid(mfn) && t == p2m_ram_shared )
+if ( mfn_valid(mfn) && p2m_is_shared(t) )
 {
 /* Does not fail with ENOMEM given the DESTROY flag */
 BUG_ON(__mem_sharing_unshare_page(
@@ -1336,7 +1336,7 @@ int relinquish_shared_pages(struct domain *d)
  * unshare.  Must succeed: we just read the old entry and
  * we hold the p2m lock.
  */
-set_rc = p2m->set_entry(p2m, _gfn(gfn), _mfn(0), PAGE_ORDER_4K,
+set_rc = p2m->set_entry(p2m, _gfn(gfn), INVALID_MFN, PAGE_ORDER_4K,
 p2m_invalid, p2m_access_rwx, -1);
 ASSERT(!set_rc);
 count += 0x10;
-- 
2.20.1


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

[Xen-devel] [PATCH v3 15/18] xen/mem_sharing: VM forking

[Tamas K Lengyel]

VM forking is the process of creating a domain with an empty memory space and a
parent domain specified from which to populate the memory when necessary. For
the new domain to be functional the VM state is copied over as part of the fork
operation (HVM params, hap allocation, etc).

Signed-off-by: Tamas K Lengyel 
---
 xen/arch/x86/hvm/hvm.c|   2 +-
 xen/arch/x86/mm/mem_sharing.c | 204 ++
 xen/arch/x86/mm/p2m.c |  11 +-
 xen/include/asm-x86/mem_sharing.h |  20 ++-
 xen/include/public/memory.h   |   5 +
 xen/include/xen/sched.h   |   1 +
 6 files changed, 239 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 5d24ceb469..3241e2a5ac 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1909,7 +1909,7 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long 
gla,
 }
 #endif
 
-/* Spurious fault? PoD and log-dirty also take this path. */
+/* Spurious fault? PoD, log-dirty and VM forking also take this path. */
 if ( p2m_is_ram(p2mt) )
 {
 rc = 1;
diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index ecbe40545d..d544801681 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -22,11 +22,13 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -36,6 +38,9 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 #include 
 
 #include "mm-locks.h"
@@ -1433,6 +1438,175 @@ static inline int mem_sharing_control(struct domain *d, 
bool enable)
 return 0;
 }
 
+/*
+ * Forking a page only gets called when the VM faults due to no entry being
+ * in the EPT for the access. Depending on the type of access we either
+ * populate the physmap with a shared entry for read-only access or
+ * fork the page if its a write access.
+ *
+ * The client p2m is already locked so we only need to lock
+ * the parent's here.
+ */
+int mem_sharing_fork_page(struct domain *d, gfn_t gfn, bool unsharing)
+{
+int rc = -ENOENT;
+shr_handle_t handle;
+struct domain *parent;
+struct p2m_domain *p2m;
+unsigned long gfn_l = gfn_x(gfn);
+mfn_t mfn, new_mfn;
+p2m_type_t p2mt;
+struct page_info *page;
+
+if ( !mem_sharing_is_fork(d) )
+return -ENOENT;
+
+parent = d->parent;
+
+if ( !unsharing )
+{
+/* For read-only accesses we just add a shared entry to the physmap */
+while ( parent )
+{
+if ( !(rc = nominate_page(parent, gfn, 0, )) )
+break;
+
+parent = parent->parent;
+}
+
+if ( !rc )
+{
+/* The client's p2m is already locked */
+struct p2m_domain *pp2m = p2m_get_hostp2m(parent);
+
+p2m_lock(pp2m);
+rc = add_to_physmap(parent, gfn_l, handle, d, gfn_l, false);
+p2m_unlock(pp2m);
+
+if ( !rc )
+return 0;
+}
+}
+
+/*
+ * If it's a write access (ie. unsharing) or if adding a shared entry to
+ * the physmap failed we'll fork the page directly.
+ */
+p2m = p2m_get_hostp2m(d);
+parent = d->parent;
+
+while ( parent )
+{
+mfn = get_gfn_query(parent, gfn_l, );
+
+if ( mfn_valid(mfn) && p2m_is_any_ram(p2mt) )
+break;
+
+put_gfn(parent, gfn_l);
+parent = parent->parent;
+}
+
+if ( !parent )
+return -ENOENT;
+
+if ( !(page = alloc_domheap_page(d, 0)) )
+{
+put_gfn(parent, gfn_l);
+return -ENOMEM;
+}
+
+new_mfn = page_to_mfn(page);
+copy_domain_page(new_mfn, mfn);
+set_gpfn_from_mfn(mfn_x(new_mfn), gfn_l);
+
+put_gfn(parent, gfn_l);
+
+return p2m->set_entry(p2m, gfn, new_mfn, PAGE_ORDER_4K, p2m_ram_rw,
+  p2m->default_access, -1);
+}
+
+static int bring_up_vcpus(struct domain *cd, struct cpupool *cpupool)
+{
+int ret;
+unsigned int i;
+
+if ( (ret = cpupool_move_domain(cd, cpupool)) )
+return ret;
+
+for ( i = 0; i < cd->max_vcpus; i++ )
+{
+if ( cd->vcpu[i] )
+continue;
+
+if ( !vcpu_create(cd, i) )
+return -EINVAL;
+}
+
+domain_update_node_affinity(cd);
+return 0;
+}
+
+static int fork_hap_allocation(struct domain *d, struct domain *cd)
+{
+int rc;
+bool preempted;
+unsigned long mb = hap_get_allocation(d);
+
+if ( mb == hap_get_allocation(cd) )
+return 0;
+
+paging_lock(cd);
+rc = hap_set_allocation(cd, mb << (20 - PAGE_SHIFT), );
+paging_unlock(cd);
+
+if ( rc )
+return rc;
+
+if ( preempted )
+return -ERESTART;
+
+return 0;
+}
+
+static void fork_tsc(struct domain *d, struct domain *cd)
+{
+uint32_t tsc_mode;
+uint32_t gtsc_khz;
+uint32_t incarnation;
+uint64_t elapsed_nsec;
+
+

[Xen-devel] [PATCH v3 08/18] x86/mem_sharing: Make add_to_physmap static and shorten name

[Tamas K Lengyel]

It's not being called from outside mem_sharing.c

Signed-off-by: Tamas K Lengyel 
---
 xen/arch/x86/mm/mem_sharing.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index 95e75ff298..84b9f130b9 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -1069,8 +1069,9 @@ err_out:
 return ret;
 }
 
-int mem_sharing_add_to_physmap(struct domain *sd, unsigned long sgfn, 
shr_handle_t sh,
-   struct domain *cd, unsigned long cgfn, bool 
lock)
+static
+int add_to_physmap(struct domain *sd, unsigned long sgfn, shr_handle_t sh,
+   struct domain *cd, unsigned long cgfn, bool lock)
 {
 struct page_info *spage;
 int ret = -EINVAL;
@@ -1582,7 +1583,7 @@ int 
mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
 sh  = mso.u.share.source_handle;
 cgfn= mso.u.share.client_gfn;
 
-rc = mem_sharing_add_to_physmap(d, sgfn, sh, cd, cgfn, true);
+rc = add_to_physmap(d, sgfn, sh, cd, cgfn, true);
 
 rcu_unlock_domain(cd);
 }
-- 
2.20.1


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

[Xen-devel] [PATCH v3 10/18] x86/mem_sharing: Replace MEM_SHARING_DEBUG with gdprintk

[Tamas K Lengyel]

Using XENLOG_ERR level since this is only used in debug paths (ie. it's
expected the user already has loglvl=all set).

Signed-off-by: Tamas K Lengyel 
---
 xen/arch/x86/mm/mem_sharing.c | 86 +--
 1 file changed, 43 insertions(+), 43 deletions(-)

diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index 0435a7f803..93e7605900 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -49,9 +49,6 @@ typedef struct pg_lock_data {
 
 static DEFINE_PER_CPU(pg_lock_data_t, __pld);
 
-#define MEM_SHARING_DEBUG(_f, _a...)  \
-debugtrace_printk("mem_sharing_debug: %s(): " _f, __func__, ##_a)
-
 /* Reverse map defines */
 #define RMAP_HASHTAB_ORDER  0
 #define RMAP_HASHTAB_SIZE   \
@@ -494,19 +491,19 @@ static int audit(void)
 /* If we can't lock it, it's definitely not a shared page */
 if ( !mem_sharing_page_lock(pg) )
 {
-MEM_SHARING_DEBUG(
-"mfn %lx in audit list, but cannot be locked (%lx)!\n",
-mfn_x(mfn), pg->u.inuse.type_info);
-errors++;
-continue;
+gdprintk(XENLOG_ERR,
+ "mfn %lx in audit list, but cannot be locked (%lx)!\n",
+ mfn_x(mfn), pg->u.inuse.type_info);
+   errors++;
+   continue;
 }
 
 /* Check if the MFN has correct type, owner and handle. */
 if ( (pg->u.inuse.type_info & PGT_type_mask) != PGT_shared_page )
 {
-MEM_SHARING_DEBUG(
-"mfn %lx in audit list, but not PGT_shared_page (%lx)!\n",
-mfn_x(mfn), pg->u.inuse.type_info & PGT_type_mask);
+gdprintk(XENLOG_ERR,
+ "mfn %lx in audit list, but not PGT_shared_page (%lx)!\n",
+ mfn_x(mfn), pg->u.inuse.type_info & PGT_type_mask);
 errors++;
 continue;
 }
@@ -514,24 +511,24 @@ static int audit(void)
 /* Check the page owner. */
 if ( page_get_owner(pg) != dom_cow )
 {
-MEM_SHARING_DEBUG("mfn %lx shared, but wrong owner %pd!\n",
-  mfn_x(mfn), page_get_owner(pg));
-errors++;
+   gdprintk(XENLOG_ERR, "mfn %lx shared, but wrong owner (%hu)!\n",
+mfn_x(mfn), page_get_owner(pg)->domain_id);
+   errors++;
 }
 
 /* Check the m2p entry */
 if ( !SHARED_M2P(get_gpfn_from_mfn(mfn_x(mfn))) )
 {
-MEM_SHARING_DEBUG("mfn %lx shared, but wrong m2p entry (%lx)!\n",
-  mfn_x(mfn), get_gpfn_from_mfn(mfn_x(mfn)));
-errors++;
+   gdprintk(XENLOG_ERR, "mfn %lx shared, but wrong m2p entry 
(%lx)!\n",
+mfn_x(mfn), get_gpfn_from_mfn(mfn_x(mfn)));
+   errors++;
 }
 
 /* Check we have a list */
 if ( (!pg->sharing) || !rmap_has_entries(pg) )
 {
-MEM_SHARING_DEBUG("mfn %lx shared, but empty gfn list!\n",
-  mfn_x(mfn));
+gdprintk(XENLOG_ERR, "mfn %lx shared, but empty gfn list!\n",
+ mfn_x(mfn));
 errors++;
 continue;
 }
@@ -550,24 +547,26 @@ static int audit(void)
 d = get_domain_by_id(g->domain);
 if ( d == NULL )
 {
-MEM_SHARING_DEBUG("Unknown dom: %hu, for PFN=%lx, MFN=%lx\n",
-  g->domain, g->gfn, mfn_x(mfn));
+gdprintk(XENLOG_ERR,
+ "Unknown dom: %hu, for PFN=%lx, MFN=%lx\n",
+ g->domain, g->gfn, mfn_x(mfn));
 errors++;
 continue;
 }
 o_mfn = get_gfn_query_unlocked(d, g->gfn, );
 if ( !mfn_eq(o_mfn, mfn) )
 {
-MEM_SHARING_DEBUG("Incorrect P2M for d=%hu, PFN=%lx."
-  "Expecting MFN=%lx, got %lx\n",
-  g->domain, g->gfn, mfn_x(mfn), mfn_x(o_mfn));
+gdprintk(XENLOG_ERR, "Incorrect P2M for d=%hu, PFN=%lx."
+ "Expecting MFN=%lx, got %lx\n",
+ g->domain, g->gfn, mfn_x(mfn), mfn_x(o_mfn));
 errors++;
 }
 if ( t != p2m_ram_shared )
 {
-MEM_SHARING_DEBUG("Incorrect P2M type for d=%hu, PFN=%lx 
MFN=%lx."
-  "Expecting t=%d, got %d\n",
-  g->domain, g->gfn, mfn_x(mfn), 
p2m_ram_shared, t);
+gdprintk(XENLOG_ERR,
+ "Incorrect P2M type for d=%hu, PFN=%lx MFN=%lx."
+ "Expecting t=%d, got %d\n",
+ g->domain, g->gfn, mfn_x(mfn), p2m_ram_shared, t);
 errors++;
 }

[Xen-devel] [PATCH v3 14/18] x86/mem_sharing: check page type count earlier

[Tamas K Lengyel]

Signed-off-by: Tamas K Lengyel 
---
 xen/arch/x86/mm/mem_sharing.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index baa3e35ded..ecbe40545d 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -652,19 +652,18 @@ static int page_make_sharable(struct domain *d,
 return -EBUSY;
 }
 
-/* Change page type and count atomically */
-if ( !get_page_and_type(page, d, PGT_shared_page) )
+/* Check if page is already typed and bail early if it is */
+if ( (page->u.inuse.type_info & PGT_count_mask) != 1 )
 {
 spin_unlock(>page_alloc_lock);
-return -EINVAL;
+return -EEXIST;
 }
 
-/* Check it wasn't already sharable and undo if it was */
-if ( (page->u.inuse.type_info & PGT_count_mask) != 1 )
+/* Change page type and count atomically */
+if ( !get_page_and_type(page, d, PGT_shared_page) )
 {
 spin_unlock(>page_alloc_lock);
-put_page_and_type(page);
-return -EEXIST;
+return -EINVAL;
 }
 
 /*
-- 
2.20.1


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

[Xen-devel] [PATCH v3 09/18] x86/mem_sharing: Convert MEM_SHARING_DESTROY_GFN to a bool

[Tamas K Lengyel]

MEM_SHARING_DESTROY_GFN is used on the 'flags' bitfield during unsharing.
However, the bitfield is not used for anything else, so just convert it to a
bool instead.

Signed-off-by: Tamas K Lengyel 
---
 xen/arch/x86/mm/mem_sharing.c | 9 -
 xen/include/asm-x86/mem_sharing.h | 5 ++---
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index 84b9f130b9..0435a7f803 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -1182,7 +1182,7 @@ err_out:
  */
 int __mem_sharing_unshare_page(struct domain *d,
unsigned long gfn,
-   uint16_t flags)
+   bool destroy)
 {
 p2m_type_t p2mt;
 mfn_t mfn;
@@ -1238,7 +1238,7 @@ int __mem_sharing_unshare_page(struct domain *d,
  * If the GFN is getting destroyed drop the references to MFN
  * (possibly freeing the page), and exit early.
  */
-if ( flags & MEM_SHARING_DESTROY_GFN )
+if ( destroy )
 {
 if ( !last_gfn )
 mem_sharing_gfn_destroy(page, d, gfn_info);
@@ -1329,9 +1329,8 @@ int relinquish_shared_pages(struct domain *d)
 mfn = p2m->get_entry(p2m, _gfn(gfn), , , 0, NULL, NULL);
 if ( mfn_valid(mfn) && p2m_is_shared(t) )
 {
-/* Does not fail with ENOMEM given the DESTROY flag */
-BUG_ON(__mem_sharing_unshare_page(
-   d, gfn, MEM_SHARING_DESTROY_GFN));
+/* Does not fail with ENOMEM given "destroy" is set to true */
+BUG_ON(__mem_sharing_unshare_page(d, gfn, true));
 /*
  * Clear out the p2m entry so no one else may try to
  * unshare.  Must succeed: we just read the old entry and
diff --git a/xen/include/asm-x86/mem_sharing.h 
b/xen/include/asm-x86/mem_sharing.h
index 13114b6346..c915fd973f 100644
--- a/xen/include/asm-x86/mem_sharing.h
+++ b/xen/include/asm-x86/mem_sharing.h
@@ -76,16 +76,15 @@ struct page_sharing_info
 unsigned int mem_sharing_get_nr_saved_mfns(void);
 unsigned int mem_sharing_get_nr_shared_mfns(void);
 
-#define MEM_SHARING_DESTROY_GFN   (1<<1)
 /* Only fails with -ENOMEM. Enforce it with a BUG_ON wrapper. */
 int __mem_sharing_unshare_page(struct domain *d,
unsigned long gfn,
-   uint16_t flags);
+   bool destroy);
 
 static inline int mem_sharing_unshare_page(struct domain *d,
unsigned long gfn)
 {
-int rc = __mem_sharing_unshare_page(d, gfn, 0);
+int rc = __mem_sharing_unshare_page(d, gfn, false);
 BUG_ON(rc && (rc != -ENOMEM));
 return rc;
 }
-- 
2.20.1


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

[Xen-devel] [PATCH v3 16/18] xen/mem_access: Use __get_gfn_type_access in set_mem_access

[Tamas K Lengyel]

Use __get_gfn_type_access instead of p2m->get_entry to trigger page-forking
when the mem_access permission is being set on a page that has not yet been
copied over from the parent.

Signed-off-by: Tamas K Lengyel 
---
 xen/arch/x86/mm/mem_access.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
index 320b9fe621..9caf08a5b2 100644
--- a/xen/arch/x86/mm/mem_access.c
+++ b/xen/arch/x86/mm/mem_access.c
@@ -303,11 +303,10 @@ static int set_mem_access(struct domain *d, struct 
p2m_domain *p2m,
 ASSERT(!ap2m);
 #endif
 {
-mfn_t mfn;
 p2m_access_t _a;
 p2m_type_t t;
-
-mfn = p2m->get_entry(p2m, gfn, , &_a, 0, NULL, NULL);
+mfn_t mfn = __get_gfn_type_access(p2m, gfn_x(gfn), , &_a,
+  P2M_ALLOC, NULL, false);
 rc = p2m->set_entry(p2m, gfn, mfn, PAGE_ORDER_4K, t, a, -1);
 }
 
-- 
2.20.1


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

[Xen-devel] [PATCH v3 00/18] VM forking

[Tamas K Lengyel]

The following series implements VM forking for Intel HVM guests to allow for
the fast creation of identical VMs without the assosciated high startup costs
of booting or restoring the VM from a savefile.

JIRA issue: https://xenproject.atlassian.net/browse/XEN-89

The main design goal with this series has been to reduce the time of creating
the VM fork as much as possible. To achieve this the VM forking process is
split into two steps:
1) forking the VM on the hypervisor side;
2) starting QEMU to handle the backed for emulated devices.

Step 1) involves creating a VM using the new "xl fork-vm" command. The
parent VM is expected to remain paused after forks are created from it (which
is different then what process forking normally entails). During this forking
operation the HVM context and VM settings are copied over to the new forked VM.
This operation is fast and it allows the forked VM to be unpaused and to be
monitored and accessed via VMI. Note however that without its device model
running (depending on what is executing in the VM) it is bound to
misbehave/crash when its trying to access devices that would be emulated by
QEMU. We anticipate that for certain use-cases this would be an acceptable
situation, in case for example when fuzzing is performed of code segments that
don't access such devices.

Step 2) involves launching QEMU to support the forked VM, which requires the
QEMU Xen savefile to be generated manually from the parent VM. This can be
accomplished simply by connecting to its QMP socket and issuing the
"xen-save-devices-state" command as documented by QEMU:
https://github.com/qemu/qemu/blob/master/docs/xen-save-devices-state.txt
Once the QEMU Xen savefile is generated the new "xl fork-launch-dm" command is
used to launch QEMU and load the specified savefile for it.

At runtime the forked VM starts running with an empty p2m which gets lazily
populated when the VM generates EPT faults, similar to how altp2m views are
populated. If the memory access is a read-only access, the p2m entry is
populated with a memory shared entry with its parent. For write memory accesses
or in case memory sharing wasn't possible (for example in case a reference is
held by a third party), a new page is allocated and the page contents are
copied over from the parent VM. Forks can be further forked if needed, thus
allowing for further memory savings.

A VM fork reset hypercall is also added that allows the fork to be reset to the
state it was just after a fork. This is an optimization for cases where the
forks are very short-lived and run without a device model, so resetting saves
some time compared to creating a brand new fork provided the fork has not
aquired a lot of memory. If the fork has a lot of memory deduplicated it is
likely going to be faster to create a new fork from scratch and destroying the
old one.

The series has been tested with both Linux and Windows VMs and functions as
expected. VM forking time has been measured to be 0.018s, device model launch
to be around 1s depending largely on the number of devices being emulated. Fork
resets have been measured to be 0.011s under the optimal circumstances.

Patches 1-2 implement changes to existing internal Xen APIs to make VM forking
possible.

Patches 3-14 are code-cleanups and adjustments of to Xen memory sharing
subsystem with no functional changes.

Patch 15 adds the hypervisor-side code implementing VM forking.

Patch 16 is integration of mem_access with forked VMs.

Patch 17 implements the VM fork reset operation hypervisor side bits.

Patch 18 adds the toolstack-side code implementing VM forking and reset.

Tamas K Lengyel (18):
  x86/hvm: introduce hvm_copy_context_and_params
  xen/x86: Make hap_get_allocation accessible
  x86/mem_sharing: make get_two_gfns take locks conditionally
  x86/mem_sharing: drop flags from mem_sharing_unshare_page
  x86/mem_sharing: don't try to unshare twice during page fault
  x86/mem_sharing: define mem_sharing_domain to hold some scattered
variables
  x86/mem_sharing: Use INVALID_MFN and p2m_is_shared in
relinquish_shared_pages
  x86/mem_sharing: Make add_to_physmap static and shorten name
  x86/mem_sharing: Convert MEM_SHARING_DESTROY_GFN to a bool
  x86/mem_sharing: Replace MEM_SHARING_DEBUG with gdprintk
  x86/mem_sharing: ASSERT that p2m_set_entry succeeds
  x86/mem_sharing: Enable mem_sharing on first memop
  x86/mem_sharing: Skip xen heap pages in memshr nominate
  x86/mem_sharing: check page type count earlier
  xen/mem_sharing: VM forking
  xen/mem_access: Use __get_gfn_type_access in set_mem_access
  x86/mem_sharing: reset a fork
  xen/tools: VM forking toolstack side

 tools/libxc/include/xenctrl.h |  13 +
 tools/libxc/xc_memshr.c   |  22 ++
 tools/libxl/libxl.h   |   7 +
 tools/libxl/libxl_create.c| 237 +-
 tools/libxl/libxl_dm.c|   2 +-
 tools/libxl/libxl_dom.c   |  83 +++--
 tools/libxl/libxl_internal.h  |   1 +
 

[Xen-devel] [PATCH v3 02/18] xen/x86: Make hap_get_allocation accessible

[Tamas K Lengyel]

During VM forking we'll copy the parent domain's parameters to the client,
including the HAP shadow memory setting that is used for storing the domain's
EPT. We'll copy this in the hypervisor instead doing it during toolstack launch
to allow the domain to start executing and unsharing memory before (or
even completely without) the toolstack.

Signed-off-by: Tamas K Lengyel 
---
 xen/arch/x86/mm/hap/hap.c | 3 +--
 xen/include/asm-x86/hap.h | 1 +
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index 3d93f3451c..c7c7ff6e99 100644
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -321,8 +321,7 @@ static void hap_free_p2m_page(struct domain *d, struct 
page_info *pg)
 }
 
 /* Return the size of the pool, rounded up to the nearest MB */
-static unsigned int
-hap_get_allocation(struct domain *d)
+unsigned int hap_get_allocation(struct domain *d)
 {
 unsigned int pg = d->arch.paging.hap.total_pages
 + d->arch.paging.hap.p2m_pages;
diff --git a/xen/include/asm-x86/hap.h b/xen/include/asm-x86/hap.h
index b94bfb4ed0..1bf07e49fe 100644
--- a/xen/include/asm-x86/hap.h
+++ b/xen/include/asm-x86/hap.h
@@ -45,6 +45,7 @@ int   hap_track_dirty_vram(struct domain *d,
 
 extern const struct paging_mode *hap_paging_get_mode(struct vcpu *);
 int hap_set_allocation(struct domain *d, unsigned int pages, bool *preempted);
+unsigned int hap_get_allocation(struct domain *d);
 
 #endif /* XEN_HAP_H */
 
-- 
2.20.1


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

[Xen-devel] [PATCH v3 12/18] x86/mem_sharing: Enable mem_sharing on first memop

[Tamas K Lengyel]

It is wasteful to require separate hypercalls to enable sharing on both the
parent and the client domain during VM forking. To speed things up we enable
sharing on the first memop in case it wasn't already enabled.

Signed-off-by: Tamas K Lengyel 
---
 xen/arch/x86/mm/mem_sharing.c | 36 +--
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index 3f36cd6bbc..b8a9228ecf 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -1412,6 +1412,24 @@ static int range_share(struct domain *d, struct domain 
*cd,
 return rc;
 }
 
+static inline int mem_sharing_control(struct domain *d, bool enable)
+{
+if ( enable )
+{
+if ( unlikely(!is_hvm_domain(d)) )
+return -ENOSYS;
+
+if ( unlikely(!hap_enabled(d)) )
+return -ENODEV;
+
+if ( unlikely(is_iommu_enabled(d)) )
+return -EXDEV;
+}
+
+d->arch.hvm.mem_sharing.enabled = enable;
+return 0;
+}
+
 int mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
 {
 int rc;
@@ -1433,10 +1451,8 @@ int 
mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
 if ( rc )
 goto out;
 
-/* Only HAP is supported */
-rc = -ENODEV;
-if ( !mem_sharing_enabled(d) )
-goto out;
+if ( !mem_sharing_enabled(d) && (rc = mem_sharing_control(d, true)) )
+return rc;
 
 switch ( mso.op )
 {
@@ -1703,18 +1719,10 @@ int mem_sharing_domctl(struct domain *d, struct 
xen_domctl_mem_sharing_op *mec)
 {
 int rc;
 
-/* Only HAP is supported */
-if ( !hap_enabled(d) )
-return -ENODEV;
-
-switch ( mec->op )
+switch( mec->op )
 {
 case XEN_DOMCTL_MEM_SHARING_CONTROL:
-rc = 0;
-if ( unlikely(is_iommu_enabled(d) && mec->u.enable) )
-rc = -EXDEV;
-else
-d->arch.hvm.mem_sharing_enabled = mec->u.enable;
+rc = mem_sharing_control(d, mec->u.enable);
 break;
 
 default:
-- 
2.20.1


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

[Xen-devel] [PATCH v3 01/18] x86/hvm: introduce hvm_copy_context_and_params

[Tamas K Lengyel]

Currently the hvm parameters are only accessible via the HVMOP hypercalls. In
this patch we introduce a new function that can copy both the hvm context and
parameters directly into a target domain. We'll use this function during VM
forking operation.

There are no functional changes to hvmop_get/set_param, only adjustments to
make the code more accessible outside the HVMOP hypercalls.

Signed-off-by: Tamas K Lengyel 
---
 xen/arch/x86/hvm/hvm.c| 241 +-
 xen/include/asm-x86/hvm/hvm.h |   2 +
 2 files changed, 152 insertions(+), 91 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 4723f5d09c..24f08d7043 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4067,16 +4067,17 @@ static int hvmop_set_evtchn_upcall_vector(
 }
 
 static int hvm_allow_set_param(struct domain *d,
-   const struct xen_hvm_param *a)
+   uint32_t index,
+   uint64_t new_value)
 {
-uint64_t value = d->arch.hvm.params[a->index];
+uint64_t value = d->arch.hvm.params[index];
 int rc;
 
 rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_set_param);
 if ( rc )
 return rc;
 
-switch ( a->index )
+switch ( index )
 {
 /* The following parameters can be set by the guest. */
 case HVM_PARAM_CALLBACK_IRQ:
@@ -4109,7 +4110,7 @@ static int hvm_allow_set_param(struct domain *d,
 if ( rc )
 return rc;
 
-switch ( a->index )
+switch ( index )
 {
 /* The following parameters should only be changed once. */
 case HVM_PARAM_VIRIDIAN:
@@ -4119,7 +4120,7 @@ static int hvm_allow_set_param(struct domain *d,
 case HVM_PARAM_NR_IOREQ_SERVER_PAGES:
 case HVM_PARAM_ALTP2M:
 case HVM_PARAM_MCA_CAP:
-if ( value != 0 && a->value != value )
+if ( value != 0 && new_value != value )
 rc = -EEXIST;
 break;
 default:
@@ -4129,49 +4130,32 @@ static int hvm_allow_set_param(struct domain *d,
 return rc;
 }
 
-static int hvmop_set_param(
-XEN_GUEST_HANDLE_PARAM(xen_hvm_param_t) arg)
+static int hvm_set_param(struct domain *d, uint32_t index, uint64_t value)
 {
 struct domain *curr_d = current->domain;
-struct xen_hvm_param a;
-struct domain *d;
-struct vcpu *v;
 int rc;
+struct vcpu *v;
 
-if ( copy_from_guest(, arg, 1) )
-return -EFAULT;
-
-if ( a.index >= HVM_NR_PARAMS )
+if ( index >= HVM_NR_PARAMS )
 return -EINVAL;
 
-/* Make sure the above bound check is not bypassed during speculation. */
-block_speculation();
-
-d = rcu_lock_domain_by_any_id(a.domid);
-if ( d == NULL )
-return -ESRCH;
-
-rc = -EINVAL;
-if ( !is_hvm_domain(d) )
-goto out;
-
-rc = hvm_allow_set_param(d, );
+rc = hvm_allow_set_param(d, index, value);
 if ( rc )
 goto out;
 
-switch ( a.index )
+switch ( index )
 {
 case HVM_PARAM_CALLBACK_IRQ:
-hvm_set_callback_via(d, a.value);
+hvm_set_callback_via(d, value);
 hvm_latch_shinfo_size(d);
 break;
 case HVM_PARAM_TIMER_MODE:
-if ( a.value > HVMPTM_one_missed_tick_pending )
+if ( value > HVMPTM_one_missed_tick_pending )
 rc = -EINVAL;
 break;
 case HVM_PARAM_VIRIDIAN:
-if ( (a.value & ~HVMPV_feature_mask) ||
- !(a.value & HVMPV_base_freq) )
+if ( (value & ~HVMPV_feature_mask) ||
+ !(value & HVMPV_base_freq) )
 rc = -EINVAL;
 break;
 case HVM_PARAM_IDENT_PT:
@@ -4181,7 +4165,7 @@ static int hvmop_set_param(
  */
 if ( !paging_mode_hap(d) || !cpu_has_vmx )
 {
-d->arch.hvm.params[a.index] = a.value;
+d->arch.hvm.params[index] = value;
 break;
 }
 
@@ -4196,7 +4180,7 @@ static int hvmop_set_param(
 
 rc = 0;
 domain_pause(d);
-d->arch.hvm.params[a.index] = a.value;
+d->arch.hvm.params[index] = value;
 for_each_vcpu ( d, v )
 paging_update_cr3(v, false);
 domain_unpause(d);
@@ -4205,23 +4189,23 @@ static int hvmop_set_param(
 break;
 case HVM_PARAM_DM_DOMAIN:
 /* The only value this should ever be set to is DOMID_SELF */
-if ( a.value != DOMID_SELF )
+if ( value != DOMID_SELF )
 rc = -EINVAL;
 
-a.value = curr_d->domain_id;
+value = curr_d->domain_id;
 break;
 case HVM_PARAM_ACPI_S_STATE:
 rc = 0;
-if ( a.value == 3 )
+if ( value == 3 )
 hvm_s3_suspend(d);
-else if ( a.value == 0 )
+else if ( value == 0 )
 hvm_s3_resume(d);
 else
 rc = -EINVAL;
 
 break;
 case HVM_PARAM_ACPI_IOPORTS_LOCATION:
-rc = pmtimer_change_ioport(d, a.value);
+rc = pmtimer_change_ioport(d, value);
 break;
 

[Xen-devel] [PATCH v3 03/18] x86/mem_sharing: make get_two_gfns take locks conditionally

[Tamas K Lengyel]

During VM forking the client lock will already be taken.

Signed-off-by: Tamas K Lengyel 
Acked-by: Andrew Coopers 
---
 xen/arch/x86/mm/mem_sharing.c | 11 ++-
 xen/include/asm-x86/p2m.h | 10 +-
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index ddf1f0f9f9..f6187403a0 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -955,7 +955,7 @@ static int share_pages(struct domain *sd, gfn_t sgfn, 
shr_handle_t sh,
 unsigned long put_count = 0;
 
 get_two_gfns(sd, sgfn, _type, NULL, ,
- cd, cgfn, _type, NULL, , 0, );
+ cd, cgfn, _type, NULL, , 0, , true);
 
 /*
  * This tricky business is to avoid two callers deadlocking if
@@ -1073,7 +1073,7 @@ err_out:
 }
 
 int mem_sharing_add_to_physmap(struct domain *sd, unsigned long sgfn, 
shr_handle_t sh,
-   struct domain *cd, unsigned long cgfn)
+   struct domain *cd, unsigned long cgfn, bool 
lock)
 {
 struct page_info *spage;
 int ret = -EINVAL;
@@ -1085,7 +1085,7 @@ int mem_sharing_add_to_physmap(struct domain *sd, 
unsigned long sgfn, shr_handle
 struct two_gfns tg;
 
 get_two_gfns(sd, _gfn(sgfn), _type, NULL, ,
- cd, _gfn(cgfn), _type, , , 0, );
+ cd, _gfn(cgfn), _type, , , 0, , lock);
 
 /* Get the source shared page, check and lock */
 ret = XENMEM_SHARING_OP_S_HANDLE_INVALID;
@@ -1162,7 +1162,8 @@ int mem_sharing_add_to_physmap(struct domain *sd, 
unsigned long sgfn, shr_handle
 err_unlock:
 mem_sharing_page_unlock(spage);
 err_out:
-put_two_gfns();
+if ( lock )
+put_two_gfns();
 return ret;
 }
 
@@ -1583,7 +1584,7 @@ int 
mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
 sh  = mso.u.share.source_handle;
 cgfn= mso.u.share.client_gfn;
 
-rc = mem_sharing_add_to_physmap(d, sgfn, sh, cd, cgfn);
+rc = mem_sharing_add_to_physmap(d, sgfn, sh, cd, cgfn, true);
 
 rcu_unlock_domain(cd);
 }
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 94285db1b4..7399c4a897 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -539,7 +539,7 @@ struct two_gfns {
 static inline void get_two_gfns(struct domain *rd, gfn_t rgfn,
 p2m_type_t *rt, p2m_access_t *ra, mfn_t *rmfn, struct domain *ld,
 gfn_t lgfn, p2m_type_t *lt, p2m_access_t *la, mfn_t *lmfn,
-p2m_query_t q, struct two_gfns *rval)
+p2m_query_t q, struct two_gfns *rval, bool lock)
 {
 mfn_t   *first_mfn, *second_mfn, scratch_mfn;
 p2m_access_t*first_a, *second_a, scratch_a;
@@ -569,10 +569,10 @@ do {\
 #undef assign_pointers
 
 /* Now do the gets */
-*first_mfn  = get_gfn_type_access(p2m_get_hostp2m(rval->first_domain),
-  gfn_x(rval->first_gfn), first_t, 
first_a, q, NULL);
-*second_mfn = get_gfn_type_access(p2m_get_hostp2m(rval->second_domain),
-  gfn_x(rval->second_gfn), second_t, 
second_a, q, NULL);
+*first_mfn  = __get_gfn_type_access(p2m_get_hostp2m(rval->first_domain),
+gfn_x(rval->first_gfn), first_t, 
first_a, q, NULL, lock);
+*second_mfn = __get_gfn_type_access(p2m_get_hostp2m(rval->second_domain),
+gfn_x(rval->second_gfn), second_t, 
second_a, q, NULL, lock);
 }
 
 static inline void put_two_gfns(struct two_gfns *arg)
-- 
2.20.1


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

[Xen-devel] [PATCH v3 04/18] x86/mem_sharing: drop flags from mem_sharing_unshare_page

[Tamas K Lengyel]

All callers pass 0 in.

Signed-off-by: Tamas K Lengyel 
Reviewed-by: Wei Liu 
---
 xen/arch/x86/hvm/hvm.c| 2 +-
 xen/arch/x86/mm/p2m.c | 5 ++---
 xen/common/memory.c   | 2 +-
 xen/include/asm-x86/mem_sharing.h | 8 +++-
 4 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 24f08d7043..38e9006c92 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1898,7 +1898,7 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long 
gla,
 if ( npfec.write_access && (p2mt == p2m_ram_shared) )
 {
 ASSERT(p2m_is_hostp2m(p2m));
-sharing_enomem = mem_sharing_unshare_page(currd, gfn, 0);
+sharing_enomem = mem_sharing_unshare_page(currd, gfn);
 rc = 1;
 goto out_put_gfn;
 }
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 3119269073..baea632acc 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -515,7 +515,7 @@ mfn_t __get_gfn_type_access(struct p2m_domain *p2m, 
unsigned long gfn_l,
  * Try to unshare. If we fail, communicate ENOMEM without
  * sleeping.
  */
-if ( mem_sharing_unshare_page(p2m->domain, gfn_l, 0) < 0 )
+if ( mem_sharing_unshare_page(p2m->domain, gfn_l) < 0 )
 mem_sharing_notify_enomem(p2m->domain, gfn_l, false);
 mfn = p2m->get_entry(p2m, gfn, t, a, q, page_order, NULL);
 }
@@ -896,8 +896,7 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, mfn_t 
mfn,
 {
 /* Do an unshare to cleanly take care of all corner cases. */
 int rc;
-rc = mem_sharing_unshare_page(p2m->domain,
-  gfn_x(gfn_add(gfn, i)), 0);
+rc = mem_sharing_unshare_page(p2m->domain, gfn_x(gfn_add(gfn, i)));
 if ( rc )
 {
 p2m_unlock(p2m);
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 309e872edf..c7d2bac452 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -352,7 +352,7 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
  * might be the only one using this shared page, and we need to
  * trigger proper cleanup. Once done, this is like any other page.
  */
-rc = mem_sharing_unshare_page(d, gmfn, 0);
+rc = mem_sharing_unshare_page(d, gmfn);
 if ( rc )
 {
 mem_sharing_notify_enomem(d, gmfn, false);
diff --git a/xen/include/asm-x86/mem_sharing.h 
b/xen/include/asm-x86/mem_sharing.h
index af2a1038b5..cf7848709f 100644
--- a/xen/include/asm-x86/mem_sharing.h
+++ b/xen/include/asm-x86/mem_sharing.h
@@ -69,10 +69,9 @@ int __mem_sharing_unshare_page(struct domain *d,
uint16_t flags);
 
 static inline int mem_sharing_unshare_page(struct domain *d,
-   unsigned long gfn,
-   uint16_t flags)
+   unsigned long gfn)
 {
-int rc = __mem_sharing_unshare_page(d, gfn, flags);
+int rc = __mem_sharing_unshare_page(d, gfn, 0);
 BUG_ON(rc && (rc != -ENOMEM));
 return rc;
 }
@@ -115,8 +114,7 @@ static inline unsigned int 
mem_sharing_get_nr_shared_mfns(void)
 return 0;
 }
 
-static inline int mem_sharing_unshare_page(struct domain *d, unsigned long gfn,
-   uint16_t flags)
+static inline int mem_sharing_unshare_page(struct domain *d, unsigned long gfn)
 {
 ASSERT_UNREACHABLE();
 return -EOPNOTSUPP;
-- 
2.20.1


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

[Xen-devel] [PATCH v3 11/18] x86/mem_sharing: ASSERT that p2m_set_entry succeeds

[Tamas K Lengyel]

Signed-off-by: Tamas K Lengyel 
---
 xen/arch/x86/mm/mem_sharing.c | 42 +--
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index 93e7605900..3f36cd6bbc 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -1117,11 +1117,19 @@ int add_to_physmap(struct domain *sd, unsigned long 
sgfn, shr_handle_t sh,
 goto err_unlock;
 }
 
+/*
+ * Must succeed, we just read the entry and hold the p2m lock
+ * via get_two_gfns.
+ */
 ret = p2m_set_entry(p2m, _gfn(cgfn), smfn, PAGE_ORDER_4K,
 p2m_ram_shared, a);
+ASSERT(!ret);
 
-/* Tempted to turn this into an assert */
-if ( ret )
+/*
+ * There is a chance we're plugging a hole where a paged out
+ * page was.
+ */
+if ( p2m_is_paging(cmfn_type) && (cmfn_type != p2m_ram_paging_out) )
 {
 mem_sharing_gfn_destroy(spage, cd, gfn_info);
 put_page_and_type(spage);
@@ -1129,29 +1137,21 @@ int add_to_physmap(struct domain *sd, unsigned long 
sgfn, shr_handle_t sh,
 else
 {
 /*
- * There is a chance we're plugging a hole where a paged out
- * page was.
+ * Further, there is a chance this was a valid page.
+ * Don't leak it.
  */
-if ( p2m_is_paging(cmfn_type) && (cmfn_type != p2m_ram_paging_out) )
+if ( mfn_valid(cmfn) )
 {
-atomic_dec(>paged_pages);
-/*
- * Further, there is a chance this was a valid page.
- * Don't leak it.
- */
-if ( mfn_valid(cmfn) )
+struct page_info *cpage = mfn_to_page(cmfn);
+
+if ( !get_page(cpage, cd) )
 {
-struct page_info *cpage = mfn_to_page(cmfn);
-
-if ( !get_page(cpage, cd) )
-{
-domain_crash(cd);
-ret = -EOVERFLOW;
-goto err_unlock;
-}
-put_page_alloc_ref(cpage);
-put_page(cpage);
+domain_crash(cd);
+ret = -EOVERFLOW;
+goto err_unlock;
 }
+put_page_alloc_ref(cpage);
+put_page(cpage);
 }
 }
 
-- 
2.20.1


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

[Xen-devel] [PATCH v3 05/18] x86/mem_sharing: don't try to unshare twice during page fault

[Tamas K Lengyel]

The page was already tried to be unshared in get_gfn_type_access. If that
didn't work, then trying again is pointless. Don't try to send vm_event again
either, simply check if there is a ring or not.

Signed-off-by: Tamas K Lengyel 
---
v3: use gprintk when the domain is going to crash
---
 xen/arch/x86/hvm/hvm.c | 28 ++--
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 38e9006c92..5d24ceb469 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -38,6 +38,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1702,11 +1703,14 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned 
long gla,
 struct domain *currd = curr->domain;
 struct p2m_domain *p2m, *hostp2m;
 int rc, fall_through = 0, paged = 0;
-int sharing_enomem = 0;
 vm_event_request_t *req_ptr = NULL;
 bool sync = false;
 unsigned int page_order;
 
+#ifdef CONFIG_MEM_SHARING
+bool sharing_enomem = false;
+#endif
+
 /* On Nested Virtualization, walk the guest page table.
  * If this succeeds, all is fine.
  * If this fails, inject a nested page fault into the guest.
@@ -1894,14 +1898,16 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned 
long gla,
 if ( p2m_is_paged(p2mt) || (p2mt == p2m_ram_paging_out) )
 paged = 1;
 
-/* Mem sharing: unshare the page and try again */
-if ( npfec.write_access && (p2mt == p2m_ram_shared) )
+#ifdef CONFIG_MEM_SHARING
+/* Mem sharing: if still shared on write access then its enomem */
+if ( npfec.write_access && p2m_is_shared(p2mt) )
 {
 ASSERT(p2m_is_hostp2m(p2m));
-sharing_enomem = mem_sharing_unshare_page(currd, gfn);
+sharing_enomem = true;
 rc = 1;
 goto out_put_gfn;
 }
+#endif
 
 /* Spurious fault? PoD and log-dirty also take this path. */
 if ( p2m_is_ram(p2mt) )
@@ -1955,19 +1961,21 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned 
long gla,
  */
 if ( paged )
 p2m_mem_paging_populate(currd, gfn);
+
+#ifdef CONFIG_MEM_SHARING
 if ( sharing_enomem )
 {
-int rv;
-
-if ( (rv = mem_sharing_notify_enomem(currd, gfn, true)) < 0 )
+if ( !vm_event_check_ring(currd->vm_event_share) )
 {
-gdprintk(XENLOG_ERR, "Domain %hu attempt to unshare "
- "gfn %lx, ENOMEM and no helper (rc %d)\n",
- currd->domain_id, gfn, rv);
+gprintk(XENLOG_ERR, "Domain %pd attempt to unshare "
+"gfn %lx, ENOMEM and no helper\n",
+currd, gfn);
 /* Crash the domain */
 rc = 0;
 }
 }
+#endif
+
 if ( req_ptr )
 {
 if ( monitor_traps(curr, sync, req_ptr) < 0 )
-- 
2.20.1


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

Re: [Xen-devel] [PATCH] tools/libxl: Reposition build_pre() logic between architectures

[Julien Grall]

Hi,

On 30/12/2019 13:38, Andrew Cooper wrote:

On 30/12/2019 13:15, Julien Grall wrote:

Hi Andrew,

On 27/12/2019 13:45, Andrew Cooper wrote:

The call to xc_domain_disable_migrate() is made only from x86, while its
handling in Xen is common.  Move it to the libxl__build_pre().

hvm_set_conf_params(), hvm_set_viridian_features(),
hvm_set_mca_capabilities(), and the altp2m logic is all in common
code (parts ifdef'd) but despite this, is all actually x86 specific.


While altp2m is only supported on x86, the concept is not
x86-specific. I am actually aware of people using altp2m on Arm,
althought the support is not upstream yet.



Move it into x86 specific code, and fold all of the
xc_hvm_param_set() calls
together into hvm_set_conf_params() in a far more coherent way.

Finally - ensure that all hypercalls have their return values checked.

No practical change in constructed domains.  Fewer useless hypercalls
now to
construct an ARM guest.


I think it would be best to keep anything that we know can be used on
arm (or new architecture) in common code. I am thinking about
"nestedhvm" and "altp2m".


Neither of those options are going to survive in this form.


Oh, it wasn't clear from the commit message. Would you mind to add a 
sentence in the commit message about it?




Also, the checks can't stay in common code.  Currently, Xen doesn't
reject bad parameters, and the toolstack doesn't check return values.
Frankly, neither of these bugs should ever have got through code review,
seeing as we were doing rather better code review by the time the ARM
port came about.


The HVM_PARAM is not great on Arm :(. It would be nice to get this fixed 
properly.




I've fixed the libxl code to check return values, but when the
hypervisor has its
"not-quite-an-XSA-because-the-guest-induced-damage-is-in-unsupported-subsystems"
bugs fixed, these hypercalls will start failing.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 4/8] x86/hyperv: setup hypercall page

[Andrew Cooper]

On 30/12/2019 13:33, Wei Liu wrote:
> On Mon, Dec 30, 2019 at 12:55:22PM +, Andrew Cooper wrote:
>> On 29/12/2019 18:33, Wei Liu wrote:
>>> @@ -71,6 +72,40 @@ const struct hypervisor_ops *__init hyperv_probe(void)
>>>  return 
>>>  }
>>>  
>>> +static void __init setup_hypercall_page(void)
>>> +{
>>> +union hv_x64_msr_hypercall_contents hypercall_msr;
>>> +
>>> +/* Unfortunately there isn't a really good way to unwind Xen to
>>> + * not use Hyper-V hooks, so panic if anything goes wrong.
>>> + *
>>> + * In practice if page allocation fails this early on it is
>>> + * unlikely we can get a working system later.
>>> + */
>>> +hv_hypercall_page = alloc_domheap_page(NULL, 0);
>>> +if ( !hv_hypercall_page )
>>> +panic("Failed to allocate Hyper-V hypercall page\n");
>>> +
>>> +hv_hypercall = __map_domain_page_global(hv_hypercall_page);
>>> +if ( !hv_hypercall )
>>> +panic("Failed to map Hyper-V hypercall page\n");
>> I really hope this doesn't actually function correctly.  This should
>> result in an NX mapping.
>>
> Ah, stupid me. I had actually looked at Xen's implementation and thought
> "wouldn't it be nice to save one page in the image".

Its 4k, and there is a lot to be said for not having random tiny
critical bits of infrastructure spread dynamically around GFN space.

> I clearly missed that __map_domain_page_global makes the page NX.

It is hidden in the depths of PAGE_HYPERVISOR.

~Andrew

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

Re: [Xen-devel] [PATCH] tools/libxl: Reposition build_pre() logic between architectures

[Andrew Cooper]

On 30/12/2019 13:15, Julien Grall wrote:
> Hi Andrew,
>
> On 27/12/2019 13:45, Andrew Cooper wrote:
>> The call to xc_domain_disable_migrate() is made only from x86, while its
>> handling in Xen is common.  Move it to the libxl__build_pre().
>>
>> hvm_set_conf_params(), hvm_set_viridian_features(),
>> hvm_set_mca_capabilities(), and the altp2m logic is all in common
>> code (parts ifdef'd) but despite this, is all actually x86 specific.
>
> While altp2m is only supported on x86, the concept is not
> x86-specific. I am actually aware of people using altp2m on Arm,
> althought the support is not upstream yet.
>
>>
>> Move it into x86 specific code, and fold all of the
>> xc_hvm_param_set() calls
>> together into hvm_set_conf_params() in a far more coherent way.
>>
>> Finally - ensure that all hypercalls have their return values checked.
>>
>> No practical change in constructed domains.  Fewer useless hypercalls
>> now to
>> construct an ARM guest.
>
> I think it would be best to keep anything that we know can be used on
> arm (or new architecture) in common code. I am thinking about
> "nestedhvm" and "altp2m".

Neither of those options are going to survive in this form.

Also, the checks can't stay in common code.  Currently, Xen doesn't
reject bad parameters, and the toolstack doesn't check return values. 
Frankly, neither of these bugs should ever have got through code review,
seeing as we were doing rather better code review by the time the ARM
port came about.

I've fixed the libxl code to check return values, but when the
hypervisor has its
"not-quite-an-XSA-because-the-guest-induced-damage-is-in-unsupported-subsystems"
bugs fixed, these hypercalls will start failing.

~Andrew

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

Re: [Xen-devel] [PATCH 5/8] x86/hyperv: provide Hyper-V hypercall functions

[Wei Liu]

On Mon, Dec 30, 2019 at 01:04:33PM +, Andrew Cooper wrote:
> On 29/12/2019 18:33, Wei Liu wrote:
> > These functions will be used later to make hypercalls to Hyper-V.
> >
> > I couldn't find reference in TLFS that Hyper-V clobbers flags and
> > r9-r11, but Linux's commit message says it does. Err on the safe side.
> >
> > Signed-off-by: Wei Liu 
> > ---
> >  xen/include/asm-x86/guest/hyperv-hypercall.h | 105 +++
> >  1 file changed, 105 insertions(+)
> >  create mode 100644 xen/include/asm-x86/guest/hyperv-hypercall.h
> >
> > diff --git a/xen/include/asm-x86/guest/hyperv-hypercall.h 
> > b/xen/include/asm-x86/guest/hyperv-hypercall.h
> > new file mode 100644
> > index 00..6017123be5
> > --- /dev/null
> > +++ b/xen/include/asm-x86/guest/hyperv-hypercall.h
> > @@ -0,0 +1,105 @@
> > +/**
> > + * asm-x86/guest/hyperv-hypercall.h
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms and conditions of the GNU General Public
> > + * License, version 2, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public
> > + * License along with this program; If not, see 
> > .
> > + *
> > + * Copyright (c) 2019 Microsoft.
> > + */
> > +
> > +#ifndef __X86_HYPERV_HYPERCALL_H__
> > +#define __X86_HYPERV_HYPERCALL_H__
> > +
> > +#include 
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +extern void *hv_hypercall;
> > +
> > +static inline uint64_t hv_do_hypercall(uint64_t control, paddr_t input, 
> > paddr_t output)
> > +{
> > +uint64_t status;
> > +
> > +if ( !hv_hypercall )
> > +return ~0ULL;
> > +
> > +asm volatile ("mov %[output], %%r8\n"
> > +  "call *%[hypercall_page]"
> > +  : "=a" (status), "+c" (control),
> > +"+d" (input) ASM_CALL_CONSTRAINT
> > +  : [output] "rm" (output),
> > +[hypercall_page] "m" (hv_hypercall)
> > +  : "cc", "memory", "r8", "r9", "r10", "r11");
> > +
> > +return status;
> > +}
> 
> Indirect calls are expensive these days due to retpoline/IBRS, and in
> this case, unnecessary.
> 
> You want something like:
> 
> asm ( ".pushsection \".text.page_aligned\", \"ax\", @progbits\n\t"
>   ".align 4096\n\t"
>   ".globl hyperv_hypercall\n\t"
>   "hyperv_hypercall:\n\t"
>   "mov -1, %rax\n\t"
>   "ret\n\t"
>   ".align 4096;\n\t" );
> 
> Which will put one page worth of space in .text.page_aligned (so it gets
> mapped executable), at a location the linker can evaluate (so you can
> use a direct call, and the disassembly will be easier to follow), which
> is initialised to the "not ready yet" code so you don't need a runtime
> check in every hypercall that you didn't get the order of initialisation
> wrong at boot.

This works for me. Thanks.

Wei.

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

Re: [Xen-devel] [PATCH 4/8] x86/hyperv: setup hypercall page

[Wei Liu]

On Mon, Dec 30, 2019 at 12:55:22PM +, Andrew Cooper wrote:
> On 29/12/2019 18:33, Wei Liu wrote:
> > @@ -71,6 +72,40 @@ const struct hypervisor_ops *__init hyperv_probe(void)
> >  return 
> >  }
> >  
> > +static void __init setup_hypercall_page(void)
> > +{
> > +union hv_x64_msr_hypercall_contents hypercall_msr;
> > +
> > +/* Unfortunately there isn't a really good way to unwind Xen to
> > + * not use Hyper-V hooks, so panic if anything goes wrong.
> > + *
> > + * In practice if page allocation fails this early on it is
> > + * unlikely we can get a working system later.
> > + */
> > +hv_hypercall_page = alloc_domheap_page(NULL, 0);
> > +if ( !hv_hypercall_page )
> > +panic("Failed to allocate Hyper-V hypercall page\n");
> > +
> > +hv_hypercall = __map_domain_page_global(hv_hypercall_page);
> > +if ( !hv_hypercall )
> > +panic("Failed to map Hyper-V hypercall page\n");
> 
> I really hope this doesn't actually function correctly.  This should
> result in an NX mapping.
> 

Ah, stupid me. I had actually looked at Xen's implementation and thought
"wouldn't it be nice to save one page in the image". I clearly missed
that __map_domain_page_global makes the page NX.

Wei.

> See feedback on the next patch for an alternative suggestion.
> 
> ~Andrew

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

Re: [Xen-devel] [PATCH] tools/libxl: Reposition build_pre() logic between architectures

[Julien Grall]

Hi Andrew,

On 27/12/2019 13:45, Andrew Cooper wrote:

The call to xc_domain_disable_migrate() is made only from x86, while its
handling in Xen is common.  Move it to the libxl__build_pre().

hvm_set_conf_params(), hvm_set_viridian_features(),
hvm_set_mca_capabilities(), and the altp2m logic is all in common
code (parts ifdef'd) but despite this, is all actually x86 specific.


While altp2m is only supported on x86, the concept is not x86-specific. 
I am actually aware of people using altp2m on Arm, althought the support 
is not upstream yet.




Move it into x86 specific code, and fold all of the xc_hvm_param_set() calls
together into hvm_set_conf_params() in a far more coherent way.

Finally - ensure that all hypercalls have their return values checked.

No practical change in constructed domains.  Fewer useless hypercalls now to
construct an ARM guest.


I think it would be best to keep anything that we know can be used on 
arm (or new architecture) in common code. I am thinking about 
"nestedhvm" and "altp2m".


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 5/8] x86/hyperv: provide Hyper-V hypercall functions

[Andrew Cooper]

On 29/12/2019 18:33, Wei Liu wrote:
> These functions will be used later to make hypercalls to Hyper-V.
>
> I couldn't find reference in TLFS that Hyper-V clobbers flags and
> r9-r11, but Linux's commit message says it does. Err on the safe side.
>
> Signed-off-by: Wei Liu 
> ---
>  xen/include/asm-x86/guest/hyperv-hypercall.h | 105 +++
>  1 file changed, 105 insertions(+)
>  create mode 100644 xen/include/asm-x86/guest/hyperv-hypercall.h
>
> diff --git a/xen/include/asm-x86/guest/hyperv-hypercall.h 
> b/xen/include/asm-x86/guest/hyperv-hypercall.h
> new file mode 100644
> index 00..6017123be5
> --- /dev/null
> +++ b/xen/include/asm-x86/guest/hyperv-hypercall.h
> @@ -0,0 +1,105 @@
> +/**
> + * asm-x86/guest/hyperv-hypercall.h
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms and conditions of the GNU General Public
> + * License, version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public
> + * License along with this program; If not, see 
> .
> + *
> + * Copyright (c) 2019 Microsoft.
> + */
> +
> +#ifndef __X86_HYPERV_HYPERCALL_H__
> +#define __X86_HYPERV_HYPERCALL_H__
> +
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +extern void *hv_hypercall;
> +
> +static inline uint64_t hv_do_hypercall(uint64_t control, paddr_t input, 
> paddr_t output)
> +{
> +uint64_t status;
> +
> +if ( !hv_hypercall )
> +return ~0ULL;
> +
> +asm volatile ("mov %[output], %%r8\n"
> +  "call *%[hypercall_page]"
> +  : "=a" (status), "+c" (control),
> +"+d" (input) ASM_CALL_CONSTRAINT
> +  : [output] "rm" (output),
> +[hypercall_page] "m" (hv_hypercall)
> +  : "cc", "memory", "r8", "r9", "r10", "r11");
> +
> +return status;
> +}

Indirect calls are expensive these days due to retpoline/IBRS, and in
this case, unnecessary.

You want something like:

asm ( ".pushsection \".text.page_aligned\", \"ax\", @progbits\n\t"
  ".align 4096\n\t"
  ".globl hyperv_hypercall\n\t"
  "hyperv_hypercall:\n\t"
  "mov -1, %rax\n\t"
  "ret\n\t"
  ".align 4096;\n\t" );

Which will put one page worth of space in .text.page_aligned (so it gets
mapped executable), at a location the linker can evaluate (so you can
use a direct call, and the disassembly will be easier to follow), which
is initialised to the "not ready yet" code so you don't need a runtime
check in every hypercall that you didn't get the order of initialisation
wrong at boot.

Alternatively, initialise to ud2 if some form of console can be reliably
be arranged to work from the very start.

~Andrew

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

Re: [Xen-devel] [PATCH] tools/libxl: Reposition build_pre() logic between architectures

[Wei Liu]

On Fri, Dec 27, 2019 at 02:50:58PM +0100, Jan Beulich wrote:
> On 27.12.2019 14:45, Andrew Cooper wrote:
> > The call to xc_domain_disable_migrate() is made only from x86, while its
> > handling in Xen is common.  Move it to the libxl__build_pre().
> > 
> > hvm_set_conf_params(), hvm_set_viridian_features(),
> > hvm_set_mca_capabilities(), and the altp2m logic is all in common
> > code (parts ifdef'd) but despite this, is all actually x86 specific.
> > 
> > Move it into x86 specific code, and fold all of the xc_hvm_param_set() calls
> > together into hvm_set_conf_params() in a far more coherent way.
> > 
> > Finally - ensure that all hypercalls have their return values checked.
> > 
> > No practical change in constructed domains.  Fewer useless hypercalls now to
> > construct an ARM guest.
> > 
> > Signed-off-by: Andrew Cooper 
> > ---
> > CC: Ian Jackson 
> > CC: Wei Liu 
> > CC: Anthony Perard 
> > CC: Jan Beulich 
> > CC: Wei Liu 
> > CC: Roger Pau Monné 
> > CC: Stefano Stabellini 
> > CC: Julien Grall 
> > CC: Volodymyr Babchuk 
> > ---
> >  tools/libxl/libxl_dom.c | 183 
> > ++--
> >  tools/libxl/libxl_x86.c | 181 
> > ++-
> 
> I'll defer to the tool stack maintainers here. Imo this file would
> better be split - one to contain stuff better falling under x86
> maintainership, and the other for everything falling in the tool
> stack realm.

(Assuming you're talking about libxl_x86.c)

That works for me.

It is unclear to me how clean that split can be though...

Wei.

> 
> Jan

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

Re: [Xen-devel] [PATCH 4/8] x86/hyperv: setup hypercall page

[Andrew Cooper]

On 29/12/2019 18:33, Wei Liu wrote:
> @@ -71,6 +72,40 @@ const struct hypervisor_ops *__init hyperv_probe(void)
>  return 
>  }
>  
> +static void __init setup_hypercall_page(void)
> +{
> +union hv_x64_msr_hypercall_contents hypercall_msr;
> +
> +/* Unfortunately there isn't a really good way to unwind Xen to
> + * not use Hyper-V hooks, so panic if anything goes wrong.
> + *
> + * In practice if page allocation fails this early on it is
> + * unlikely we can get a working system later.
> + */
> +hv_hypercall_page = alloc_domheap_page(NULL, 0);
> +if ( !hv_hypercall_page )
> +panic("Failed to allocate Hyper-V hypercall page\n");
> +
> +hv_hypercall = __map_domain_page_global(hv_hypercall_page);
> +if ( !hv_hypercall )
> +panic("Failed to map Hyper-V hypercall page\n");

I really hope this doesn't actually function correctly.  This should
result in an NX mapping.

See feedback on the next patch for an alternative suggestion.

~Andrew

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

Re: [Xen-devel] [PATCH] tools/libxl: Reposition build_pre() logic between architectures

[Wei Liu]

On Fri, Dec 27, 2019 at 01:45:16PM +, Andrew Cooper wrote:
> The call to xc_domain_disable_migrate() is made only from x86, while its
> handling in Xen is common.  Move it to the libxl__build_pre().
> 
> hvm_set_conf_params(), hvm_set_viridian_features(),
> hvm_set_mca_capabilities(), and the altp2m logic is all in common
> code (parts ifdef'd) but despite this, is all actually x86 specific.
> 
> Move it into x86 specific code, and fold all of the xc_hvm_param_set() calls
> together into hvm_set_conf_params() in a far more coherent way.
> 
> Finally - ensure that all hypercalls have their return values checked.
> 
> No practical change in constructed domains.  Fewer useless hypercalls now to
> construct an ARM guest.
> 
> Signed-off-by: Andrew Cooper 

I'm fine with moving the code. AIUI Arm guests also need to set at least
one hvm param for callback vector, but that's already handled in
libxl_arm.c.

As far as I can tell the code is correct, there is no code in between
the moved code that depends on some of the fields being set in a
specific order, so:

Acked-by: Wei Liu 

Wei.

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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail REGR. vs. 
145025

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

version targeted for testing:
 xen  3a13ac3ad4d3ef399fe2c85fb09fcb7ab1cdd140
baseline version:
 xen  0cd791c499bdc698d14a24050ec56d60b45732e0

Last test of basis   145025  2019-12-20 13:58:10 Z9 days
Failing since145058  2019-12-21 07:15:37 Z9 days   22 attempts
Testing same since   145321  2019-12-28 07:51:14 Z2 days5 attempts


People who touched