[Xen-devel] [linux-linus test] 115182: regressions - FAIL
flight 115182 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/115182/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail REGR. vs. 114682 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 114682 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 114682 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 114682 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 114682 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 114682 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 114682 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail like 114682 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 114682 test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-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-i386-libvirt-qcow2 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-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-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-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-libvirt-xsm 13 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-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass version targeted for testing: linux6cff0a118f23b98c604a3604ea9de11338e24fbe baseline version: linuxebe6e90ccc6679cb01d2b280e4b61e6092d4bedb Last test of basis 114682 2017-10-18 09:54:11 Z6 days Failing since114781 2017-10-20 01:00:47 Z5 days9 attempts Testing same since 115170 2017-10-24 02:47:58 Z1 days2 attempts 316 people touched revisions under test, not listing them all jobs: build-amd64-xsm pass build-armhf-xsm pass build-i386-xsm pass build-amd64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-armhf-pvops
Re: [Xen-devel] [PATCH] libxc: don't fail domain creation when unpacking initrd fails
On 10/16/17 11:48 AM, Andrew Cooper wrote: > On 16/10/17 17:19, Jan Beulich wrote: > On 16.10.17 at 17:45,wrote: > > I've been bitten by this issue several times before, and a fix would be > nice. Same here. > > IMO, the toolstack should not be making assumptions about the initrd, > and shouldn't be touching it. It is the users responsibility to provide > an initrd which its kernel can read. > > Furthermore, leaving the decompression to the kernel reduces the dom0 > attack surface. This. So many this. I do recall bringing this up at a meet up a while back and the concern was breaking someone's workflow. Maybe we can put a warning that the behavior is deprecated for X number of releases before deleting it. -- Doug Goldstein ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen: fix booting ballooned down hvm guest
On Tue, Oct 24, 2017 at 02:59:00PM +, HW42 wrote: > I think you really should allow pseudonymous contributions. But in my > case my nickname is anyway linked to my legal name so fell free to use: > Simon GaiserI personally always have difficulties with pseudos in emails, I think it's just a matter of respect for all other contributors. It's easier to write to a contributor using "Simon what's your opinion" than "XW;75_@!XVn what's your opinion". Just like your boss probably doesn't call you "HW42", it's understandable that the people having to deal with your work might prefer to call you with your real name as well. Willy ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V3 2/29] VIOMMU: Add vIOMMU helper functions to create, destroy vIOMMU instance
On 2017年10月19日 16:47, Roger Pau Monné wrote: > For all platforms supporting HVM, for PV I don't think it makes sense. > Since AFAIK ARM guest type is also HVM I would rather introduce this > field in the hvm_domain structure rather than the generic domain > structure. > This sounds reasonable. > You might want to wait for feedback from others regarding this issue. > I discussed with Julien before. He hoped no to add viommu code for ARM first.So struct hvm_domain seems to be better place since it's arch specific definition and only add struct viommu for struct hvm_domain of x86. -- Best regards Tianyu Lan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] x86/vpmu: Remove unnecessary call to do_interrupt()
This call was left during PVHv1 removal (commit 33e5c32559e1 ("x86: remove PVHv1 code")): -if ( is_pvh_vcpu(sampling) && - !(vpmu_mode & XENPMU_MODE_ALL) && +if ( !(vpmu_mode & XENPMU_MODE_ALL) && !vpmu->arch_vpmu_ops->do_interrupt(regs) ) return; As result of this extra call VPMU no longer works for PV guests on Intel because we effectively lose value of MSR_CORE_PERF_GLOBAL_STATUS. Signed-off-by: Boris Ostrovsky--- This should also go into 4.9 xen/arch/x86/cpu/vpmu.c | 4 1 file changed, 4 deletions(-) diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c index fd2fcac..7baf461 100644 --- a/xen/arch/x86/cpu/vpmu.c +++ b/xen/arch/x86/cpu/vpmu.c @@ -227,10 +227,6 @@ void vpmu_do_interrupt(struct cpu_user_regs *regs) if ( !vpmu->xenpmu_data ) return; -if ( !(vpmu_mode & XENPMU_MODE_ALL) && - !vpmu->arch_vpmu_ops->do_interrupt(regs) ) -return; - if ( vpmu_is_set(vpmu, VPMU_CACHED) ) return; -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/boot: fix MB2 header to require EFI BS
On Tue, Oct 24, 2017 at 10:40:26PM +0100, Andrew Cooper wrote: > On 24/10/2017 22:11, Daniel Kiper wrote: > > On Tue, Oct 24, 2017 at 09:22:20PM +0100, Andrew Cooper wrote: > >> On 24/10/17 21:08, Daniel Kiper wrote: > >>> On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote: > The EFI multiboot2 entry point currently requires EFI BootServices to > not have been exited however the header currently tells the boot > loader that Xen optionally supports EFI BootServices having been exited. > With this change Xen properly advertises that EFI must not be exited > allowing the boot loader to report an error that it cannot boot Xen if > it is unable to meet its needs. > > Signed-off-by: Doug Goldstein> --- > > This should likely be applied against Xen 4.9 and Xen 4.10 as well as > staging. I am trying to get multiboot2 support for iPXE and upstream > is concerned that leaving EFI BootServices enabled will not be > compatible with their aims to support Secure Boot. So when I build > >>> Hmmm... What are exact arguments for that? How do they implement e.g. > >>> chain loading then? What about the shim support? > >>> > my iPXE without support for passing on Boot Services, Xen will be > loaded by iPXE but then it will fall down with "ERR: Bootloader > shutdown EFI x64 boot services!" implying that this is required. By > having Xen expose in its header that its required it allows me to > handle the situation gracefully in iPXE. > > To quote the multiboot2 spec exact: > > "This tag indicates that payload supports starting without terminating > boot services." > > Unfortunately the spec is a bit vague and how I am reading it is: > - no tag = exit boot services in the boot loader > - tag present marked optional = boot loader can or cannot exit boot > services > - tag present marked required = boot loader cannot exit boot services > >>> NACK, please take a look at section 3.1.4, Multiboot2 information request > >>> in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the > >>> bootloader > >>> than you think. > >> The meaning of tag, if understood by Grub, is "don't exit boot services > >> before passing control". > > Yep. > > > >> The tag is currently marked as optional, which means Grub is free to > >> ignore it if it doesn't understand it, resulting in EBS being called > >> before passing control. > > Yep, but you are forgetting about legacy BIOS platforms with old GRUB2. > > Right now it is possible to boot Xen via Multiboot2 in such configs. > > If you set this flag to REQUIRED then old GRUB2 on BIOS platforms will > > not boot Xen in such cases. If we do not care about that then OK. However, > > then I would request additional line or so to the commit message saying > > that such configs are broken deliberately because... > > Such older cases wouldn't boot either, because Xen would bail out saying > "I didn't retain BS like I need". Nope, you should remember that legacy entry point (__start) will be used then. > >> Xen cannot cope with with EBS having been called, so must not be passed > >> control under those circumstances. > > Even with REQUIRED flag there is no guarantee that booloader will do > > what Xen asks. So, it has to check the boot services presence anyway. > > And it does. > > Indeed, and rightly so. > > The difference between Grub bailing out with an error, and Xen bailing > out with an error, is that one still leaves you at a grub prompt, while > one locks your system up until you remove some electrons from it. > > Setting the REQUIRED flag appears to be strictly better behaviour from > the users point of view. Yep, I put a solution proposal for that issue in other email. Daniel ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/boot: fix MB2 header to require EFI BS
On Tue, Oct 24, 2017 at 03:49:10PM -0500, Doug Goldstein wrote: > On 10/24/17 3:22 PM, Andrew Cooper wrote: > > On 24/10/17 21:08, Daniel Kiper wrote: > >> On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote: > >>> The EFI multiboot2 entry point currently requires EFI BootServices to > >>> not have been exited however the header currently tells the boot > >>> loader that Xen optionally supports EFI BootServices having been exited. > >>> With this change Xen properly advertises that EFI must not be exited > >>> allowing the boot loader to report an error that it cannot boot Xen if > >>> it is unable to meet its needs. > >>> > >>> Signed-off-by: Doug Goldstein> >>> --- > >>> > >>> This should likely be applied against Xen 4.9 and Xen 4.10 as well as > >>> staging. I am trying to get multiboot2 support for iPXE and upstream > >>> is concerned that leaving EFI BootServices enabled will not be > >>> compatible with their aims to support Secure Boot. So when I build > >> Hmmm... What are exact arguments for that? How do they implement e.g. > >> chain loading then? What about the shim support? > >> > >>> my iPXE without support for passing on Boot Services, Xen will be > >>> loaded by iPXE but then it will fall down with "ERR: Bootloader > >>> shutdown EFI x64 boot services!" implying that this is required. By > >>> having Xen expose in its header that its required it allows me to > >>> handle the situation gracefully in iPXE. > >>> > >>> To quote the multiboot2 spec exact: > >>> > >>> "This tag indicates that payload supports starting without terminating > >>> boot services." > >>> > >>> Unfortunately the spec is a bit vague and how I am reading it is: > >>> - no tag = exit boot services in the boot loader > >>> - tag present marked optional = boot loader can or cannot exit boot > >>> services > >>> - tag present marked required = boot loader cannot exit boot services > >> NACK, please take a look at section 3.1.4, Multiboot2 information request > >> in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the > >> bootloader > >> than you think. > > > > The meaning of tag, if understood by Grub, is "don't exit boot services > > before passing control". > > > > The tag is currently marked as optional, which means Grub is free to > > ignore it if it doesn't understand it, resulting in EBS being called > > before passing control. > > > > Xen cannot cope with with EBS having been called, so must not be passed > > control under those circumstances. > > > > Doug's patch marks it as non-optional which, by that section above, > > requires Grub to fail with an error rather than proceeding, if it does > > not understand the tag. > > > > > > By my reading, Doug's patch looks correct. > > > > How does your interpretation of the spec differ? > > > > ~Andrew > > > > So I've been sitting here reading it for a bit. I'm guessing what Daniel > is arguing is that the spec says that the boot loader MUST understand a > tag if its marked as required and does not have to understand it if its > marked as optional. The next sentence then seems to be a total escape > hatch because it seems to imply that the boot loader doesn't have to > respect any tag regardless of its required or optional settings. Which > seems to defeat the purpose of having any info requests at all. And > results in no guarantees that if your binary requires something that it > will get it before being executed. And therefore requires a binary to > support all cases always. Sadly you are right here. > If that's truly the case you are arguing for Daniel then this whole spec > really has too big of a loophole to be safely considered as useful. I > know that's a bit harsh but as more tags are added over time the matrix > of required support will snowball. I think that we can use another bit from flags and if it set then interpret it as: I (image/OS/Xen/...) have to have this data. If you (the booloader) are not able to provide it then do not start me and fail nicely, e.g. display user prompt. This bit should be checked only if current bit is in REQUIRED state. Otherwise, if new bit is in REQUIRED state and current one is in OPTIONAL state then the bootloader should complain. This way older GRUB will fail if it sees an unsupported option and newer one will always provide required data to the loaded image. And by the way, we should check unused flags bits for 1. If one of it is set then the bootloader should fail. Right now at least GRUB2 does not do that. Does it make sense? Daniel ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [qemu-mainline test] 115183: regressions - FAIL
flight 115183 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/115183/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-i386-xsm6 xen-buildfail REGR. vs. 114507 build-i3866 xen-buildfail REGR. vs. 114507 build-amd64-xsm 6 xen-buildfail REGR. vs. 114507 build-amd64 6 xen-buildfail REGR. vs. 114507 build-armhf-xsm 6 xen-buildfail REGR. vs. 114507 build-armhf 6 xen-buildfail REGR. vs. 114507 Tests which did not succeed, but are not blocking: test-amd64-i386-libvirt-qcow2 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64 1 build-check(1)blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-intel 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win10-i386 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-amd64-pair 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit2 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win10-i386 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-amd64-pygrub 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qcow2 1 build-check(1) blocked n/a test-amd64-amd64-amd64-pvgrub 1 build-check(1) blocked n/a test-armhf-armhf-xl-arndale 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-xl-credit2 1 build-check(1) blocked n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-i386-xl-xsm1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 1 build-check(1)blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-raw1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-amd64-i386-pvgrub 1 build-check(1) blocked n/a build-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvhv2-intel 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-xsm 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-intel 1 build-check(1) blocked n/a test-armhf-armhf-xl-vhd 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl 1 build-check(1) blocked n/a test-amd64-i386-pair 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvhv2-amd 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-amd 1 build-check(1) blocked n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) blocked n/a test-armhf-armhf-xl-rtds 1
Re: [Xen-devel] [PATCH] x86/boot: fix MB2 header to require EFI BS
On Tue, Oct 24, 2017 at 03:28:52PM -0500, Doug Goldstein wrote: > On 10/24/17 3:08 PM, Daniel Kiper wrote: > > On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote: > >> The EFI multiboot2 entry point currently requires EFI BootServices to > >> not have been exited however the header currently tells the boot > >> loader that Xen optionally supports EFI BootServices having been exited. > >> With this change Xen properly advertises that EFI must not be exited > >> allowing the boot loader to report an error that it cannot boot Xen if > >> it is unable to meet its needs. > >> > >> Signed-off-by: Doug Goldstein> >> --- > >> > >> This should likely be applied against Xen 4.9 and Xen 4.10 as well as > >> staging. I am trying to get multiboot2 support for iPXE and upstream > >> is concerned that leaving EFI BootServices enabled will not be > >> compatible with their aims to support Secure Boot. So when I build > > > > Hmmm... What are exact arguments for that? How do they implement e.g. > > chain loading then? What about the shim support? > > Look they have concerns about it. As we've talked about this in the past If I do something I like to know why I have to do it. > and I encourage you communicate with them. You are the author of the I remember but, sorry, IIRC, I heard just only vague statements like that. So, I would like to know exact reasons finally. And I hoped that they told you more then simple "NO". > multiboot2 spec. I'm just trying to do my best to PXE boot Xen on EFI > systems and make all upstreams (Xen & iPXE) happy. Once again, I am happy to help. Though I have to know why I have to do this or that. No more no less. > >> Unfortunately the spec is a bit vague and how I am reading it is: > >> - no tag = exit boot services in the boot loader > >> - tag present marked optional = boot loader can or cannot exit boot > >> services > >> - tag present marked required = boot loader cannot exit boot services > > > > NACK, please take a look at section 3.1.4, Multiboot2 information request > > in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the > > bootloader > > than you think. > > I still don't see any issue with my interpretation based on what you > pointed me to. There's a hole here with what Xen asks for of the boot > loader to do. > > The boot loader is told that Xen optionally supports the boot loader not > exiting boot services when in fact Xen requires the boot loader to not > exit boot services. Somehow we need to convey this to the boot loader. Sorry, maybe I was too vague this time. Please look at my replay to Andrew. It should help. Daniel ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/boot: fix MB2 header to require EFI BS
On 24/10/2017 22:11, Daniel Kiper wrote: > On Tue, Oct 24, 2017 at 09:22:20PM +0100, Andrew Cooper wrote: >> On 24/10/17 21:08, Daniel Kiper wrote: >>> On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote: The EFI multiboot2 entry point currently requires EFI BootServices to not have been exited however the header currently tells the boot loader that Xen optionally supports EFI BootServices having been exited. With this change Xen properly advertises that EFI must not be exited allowing the boot loader to report an error that it cannot boot Xen if it is unable to meet its needs. Signed-off-by: Doug Goldstein--- This should likely be applied against Xen 4.9 and Xen 4.10 as well as staging. I am trying to get multiboot2 support for iPXE and upstream is concerned that leaving EFI BootServices enabled will not be compatible with their aims to support Secure Boot. So when I build >>> Hmmm... What are exact arguments for that? How do they implement e.g. >>> chain loading then? What about the shim support? >>> my iPXE without support for passing on Boot Services, Xen will be loaded by iPXE but then it will fall down with "ERR: Bootloader shutdown EFI x64 boot services!" implying that this is required. By having Xen expose in its header that its required it allows me to handle the situation gracefully in iPXE. To quote the multiboot2 spec exact: "This tag indicates that payload supports starting without terminating boot services." Unfortunately the spec is a bit vague and how I am reading it is: - no tag = exit boot services in the boot loader - tag present marked optional = boot loader can or cannot exit boot services - tag present marked required = boot loader cannot exit boot services >>> NACK, please take a look at section 3.1.4, Multiboot2 information request >>> in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the >>> bootloader >>> than you think. >> The meaning of tag, if understood by Grub, is "don't exit boot services >> before passing control". > Yep. > >> The tag is currently marked as optional, which means Grub is free to >> ignore it if it doesn't understand it, resulting in EBS being called >> before passing control. > Yep, but you are forgetting about legacy BIOS platforms with old GRUB2. > Right now it is possible to boot Xen via Multiboot2 in such configs. > If you set this flag to REQUIRED then old GRUB2 on BIOS platforms will > not boot Xen in such cases. If we do not care about that then OK. However, > then I would request additional line or so to the commit message saying > that such configs are broken deliberately because... Such older cases wouldn't boot either, because Xen would bail out saying "I didn't retain BS like I need". > >> Xen cannot cope with with EBS having been called, so must not be passed >> control under those circumstances. > Even with REQUIRED flag there is no guarantee that booloader will do > what Xen asks. So, it has to check the boot services presence anyway. > And it does. Indeed, and rightly so. The difference between Grub bailing out with an error, and Xen bailing out with an error, is that one still leaves you at a grub prompt, while one locks your system up until you remove some electrons from it. Setting the REQUIRED flag appears to be strictly better behaviour from the users point of view. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC 0/4] TEE mediator framework + OP-TEE mediator
On Tue, 24 Oct 2017, Volodymyr Babchuk wrote: > > > > >Approach in this RFC is much simpler. Few hooks in arch code + > > > > >additional > > > > >subsystem, which can be easily turned off. > > > > > > > > Stefano do you have any opinion on this discussion? > > > > We need to start somewhere, and I think this series could be a decent > > starting point. > > > > I think it is OK to have a small SMC filter in Xen. What Volodymyr is > > suggesting looks reasonable for now. As the code grows, we might found > > ourselves in the situation where we'll have to introduce stubdoms for > > TEE virtualization/emulation, and I think that's OK. Possibly, we'll > > have a "fast path" in Xen, only for filtering and small manipulations, > > and a "slow path" in the stubdom when more complex actions are > > necessary. > This sounds a bit tricky, actually. If I got you right, you are > proposing to split mediator into two parts. Only benefit I can see > there - fast calls to OP-TEE from Dom0. That probably can work, but I > need to consider all consequences... OK :-) > > For this series, I think we need a way to specify which domains can talk > > to TEE, so that we can only allow it for a specific subset of DomUs. I > > would probably use XSM for that. > I am afraid, this is not possible. As other domains aren't 1:1 mapped, > I need to have special translation code in mediator. Actually, I'm > writing it rigth now to test my changes in OP-TEE. But event this is > not enought for decent OP-TEE support. > What can be done right now: 100% Dom0-only support with vanilla > OP-TEE (i.e. no virtualization support in OP-TEE is needed). This is > even simplier task, so I can throw out some code from this patch > series. On other hand, in the future this will lead to sutiation when > two mediators for the same TEE shall be supported: one, simple, in > XEN, another, fully-functional in stubdom. I think it is fine to support OP-TEE only in Dom0 to begin with. Ideally, it would be in Dom0 for convenience and speed and the OP-TEE capability would be specified as an XSM label. Ideally, it would not be only in Dom0 because it is tied to the 1:1 map, but I understand now that it is a requirement. I still think that the XSM label would be good to have even if today it cannot be changed as only Dom0 is 1:1. > > For the long term, I think both Volodymyr and us as maintainers need to > > be prepared to introduce stubdoms for TEE emulation. It will most > > probably happen as the feature-set grows. However, this small TEE > > framework in Xen could still be useful, and could be the basis for > > forwarding TEE requests to a stubdom for evaluation: maybe not all calls > > need to be forwarded to the stubdom, some of them could go directly to > > the firmware and this is where this series comes in. > > > > What do you think? > Hmm... I can't imagine how this can work for OP-TEE. In OP-TEE > protocol, there is a number of "fast" (in SMCCC terms) service calls, > which called mostly during initialization (to probe UID and version, > to get shared region location, to exchange caps and so on) and one > "yielding" (again, SMCCC term) call for actuall TEE tasks. The later > one passes arguments in command buffer (not in registers), it can > cause so-called RPC returns (when OP-TEE asks Normal World to perform > certain work). Most of the mediator code will be devoted to handle > this one type of call. So, I don't see benefit in splitting mediator > between XEN and stubdom. At least for OP-TEE. Maybe this is not true > for other TEEs. > Looks like Google Trusty employs another approach for NW<->SW > communication, maybe it can work in theirs case... It is possible to get a request from the command buffer in Xen, then forward it to the stubdom over a separate ring. This is pretty much how QEMU works on x86 to do emulation: IO accesses are trapped in Xen, then Xen issues requests to QEMU over a special ring. Xen doesn't need to forward to QEMU all requests: some of them could be handled directly in Xen. But maybe this model doesn't actually make sense for OP-TEE? Would it make sense to extract a request from the ring in Xen then evaluate whether it should be handled in Xen, forwarded to the firmware, or forwarded to a stubdom? For example, would it be possible to forward to firmware accesses to certain TAs while forwarding to a stubdom accesses to other TAs? ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/boot: fix MB2 header to require EFI BS
On Tue, Oct 24, 2017 at 09:22:20PM +0100, Andrew Cooper wrote: > On 24/10/17 21:08, Daniel Kiper wrote: > > On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote: > >> The EFI multiboot2 entry point currently requires EFI BootServices to > >> not have been exited however the header currently tells the boot > >> loader that Xen optionally supports EFI BootServices having been exited. > >> With this change Xen properly advertises that EFI must not be exited > >> allowing the boot loader to report an error that it cannot boot Xen if > >> it is unable to meet its needs. > >> > >> Signed-off-by: Doug Goldstein> >> --- > >> > >> This should likely be applied against Xen 4.9 and Xen 4.10 as well as > >> staging. I am trying to get multiboot2 support for iPXE and upstream > >> is concerned that leaving EFI BootServices enabled will not be > >> compatible with their aims to support Secure Boot. So when I build > > Hmmm... What are exact arguments for that? How do they implement e.g. > > chain loading then? What about the shim support? > > > >> my iPXE without support for passing on Boot Services, Xen will be > >> loaded by iPXE but then it will fall down with "ERR: Bootloader > >> shutdown EFI x64 boot services!" implying that this is required. By > >> having Xen expose in its header that its required it allows me to > >> handle the situation gracefully in iPXE. > >> > >> To quote the multiboot2 spec exact: > >> > >> "This tag indicates that payload supports starting without terminating > >> boot services." > >> > >> Unfortunately the spec is a bit vague and how I am reading it is: > >> - no tag = exit boot services in the boot loader > >> - tag present marked optional = boot loader can or cannot exit boot > >> services > >> - tag present marked required = boot loader cannot exit boot services > > NACK, please take a look at section 3.1.4, Multiboot2 information request > > in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the > > bootloader > > than you think. > > The meaning of tag, if understood by Grub, is "don't exit boot services > before passing control". Yep. > The tag is currently marked as optional, which means Grub is free to > ignore it if it doesn't understand it, resulting in EBS being called > before passing control. Yep, but you are forgetting about legacy BIOS platforms with old GRUB2. Right now it is possible to boot Xen via Multiboot2 in such configs. If you set this flag to REQUIRED then old GRUB2 on BIOS platforms will not boot Xen in such cases. If we do not care about that then OK. However, then I would request additional line or so to the commit message saying that such configs are broken deliberately because... > Xen cannot cope with with EBS having been called, so must not be passed > control under those circumstances. Even with REQUIRED flag there is no guarantee that booloader will do what Xen asks. So, it has to check the boot services presence anyway. And it does. Daniel ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/boot: fix MB2 header to require EFI BS
On 10/24/17 3:22 PM, Andrew Cooper wrote: > On 24/10/17 21:08, Daniel Kiper wrote: >> On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote: >>> The EFI multiboot2 entry point currently requires EFI BootServices to >>> not have been exited however the header currently tells the boot >>> loader that Xen optionally supports EFI BootServices having been exited. >>> With this change Xen properly advertises that EFI must not be exited >>> allowing the boot loader to report an error that it cannot boot Xen if >>> it is unable to meet its needs. >>> >>> Signed-off-by: Doug Goldstein>>> --- >>> >>> This should likely be applied against Xen 4.9 and Xen 4.10 as well as >>> staging. I am trying to get multiboot2 support for iPXE and upstream >>> is concerned that leaving EFI BootServices enabled will not be >>> compatible with their aims to support Secure Boot. So when I build >> Hmmm... What are exact arguments for that? How do they implement e.g. >> chain loading then? What about the shim support? >> >>> my iPXE without support for passing on Boot Services, Xen will be >>> loaded by iPXE but then it will fall down with "ERR: Bootloader >>> shutdown EFI x64 boot services!" implying that this is required. By >>> having Xen expose in its header that its required it allows me to >>> handle the situation gracefully in iPXE. >>> >>> To quote the multiboot2 spec exact: >>> >>> "This tag indicates that payload supports starting without terminating >>> boot services." >>> >>> Unfortunately the spec is a bit vague and how I am reading it is: >>> - no tag = exit boot services in the boot loader >>> - tag present marked optional = boot loader can or cannot exit boot services >>> - tag present marked required = boot loader cannot exit boot services >> NACK, please take a look at section 3.1.4, Multiboot2 information request >> in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the >> bootloader >> than you think. > > The meaning of tag, if understood by Grub, is "don't exit boot services > before passing control". > > The tag is currently marked as optional, which means Grub is free to > ignore it if it doesn't understand it, resulting in EBS being called > before passing control. > > Xen cannot cope with with EBS having been called, so must not be passed > control under those circumstances. > > Doug's patch marks it as non-optional which, by that section above, > requires Grub to fail with an error rather than proceeding, if it does > not understand the tag. > > > By my reading, Doug's patch looks correct. > > How does your interpretation of the spec differ? > > ~Andrew > So I've been sitting here reading it for a bit. I'm guessing what Daniel is arguing is that the spec says that the boot loader MUST understand a tag if its marked as required and does not have to understand it if its marked as optional. The next sentence then seems to be a total escape hatch because it seems to imply that the boot loader doesn't have to respect any tag regardless of its required or optional settings. Which seems to defeat the purpose of having any info requests at all. And results in no guarantees that if your binary requires something that it will get it before being executed. And therefore requires a binary to support all cases always. If that's truly the case you are arguing for Daniel then this whole spec really has too big of a loophole to be safely considered as useful. I know that's a bit harsh but as more tags are added over time the matrix of required support will snowball. -- Doug Goldstein ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/boot: fix MB2 header to require EFI BS
On 10/24/17 3:08 PM, Daniel Kiper wrote: > On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote: >> >> Unfortunately the spec is a bit vague and how I am reading it is: >> - no tag = exit boot services in the boot loader >> - tag present marked optional = boot loader can or cannot exit boot services >> - tag present marked required = boot loader cannot exit boot services > > NACK, please take a look at section 3.1.4, Multiboot2 information request > in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the bootloader > than you think. In fact since there is some confusion here then could you possibly expand some of the sections with examples to help clarify? -- Doug Goldstein ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/boot: fix MB2 header to require EFI BS
On 10/24/17 3:08 PM, Daniel Kiper wrote: > On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote: >> The EFI multiboot2 entry point currently requires EFI BootServices to >> not have been exited however the header currently tells the boot >> loader that Xen optionally supports EFI BootServices having been exited. >> With this change Xen properly advertises that EFI must not be exited >> allowing the boot loader to report an error that it cannot boot Xen if >> it is unable to meet its needs. >> >> Signed-off-by: Doug Goldstein>> --- >> >> This should likely be applied against Xen 4.9 and Xen 4.10 as well as >> staging. I am trying to get multiboot2 support for iPXE and upstream >> is concerned that leaving EFI BootServices enabled will not be >> compatible with their aims to support Secure Boot. So when I build > > Hmmm... What are exact arguments for that? How do they implement e.g. > chain loading then? What about the shim support? Look they have concerns about it. As we've talked about this in the past and I encourage you communicate with them. You are the author of the multiboot2 spec. I'm just trying to do my best to PXE boot Xen on EFI systems and make all upstreams (Xen & iPXE) happy. >> >> Unfortunately the spec is a bit vague and how I am reading it is: >> - no tag = exit boot services in the boot loader >> - tag present marked optional = boot loader can or cannot exit boot services >> - tag present marked required = boot loader cannot exit boot services > > NACK, please take a look at section 3.1.4, Multiboot2 information request > in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the bootloader > than you think. > I still don't see any issue with my interpretation based on what you pointed me to. There's a hole here with what Xen asks for of the boot loader to do. The boot loader is told that Xen optionally supports the boot loader not exiting boot services when in fact Xen requires the boot loader to not exit boot services. Somehow we need to convey this to the boot loader. -- Doug Goldstein ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/boot: fix MB2 header to require EFI BS
On 24/10/17 21:08, Daniel Kiper wrote: > On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote: >> The EFI multiboot2 entry point currently requires EFI BootServices to >> not have been exited however the header currently tells the boot >> loader that Xen optionally supports EFI BootServices having been exited. >> With this change Xen properly advertises that EFI must not be exited >> allowing the boot loader to report an error that it cannot boot Xen if >> it is unable to meet its needs. >> >> Signed-off-by: Doug Goldstein>> --- >> >> This should likely be applied against Xen 4.9 and Xen 4.10 as well as >> staging. I am trying to get multiboot2 support for iPXE and upstream >> is concerned that leaving EFI BootServices enabled will not be >> compatible with their aims to support Secure Boot. So when I build > Hmmm... What are exact arguments for that? How do they implement e.g. > chain loading then? What about the shim support? > >> my iPXE without support for passing on Boot Services, Xen will be >> loaded by iPXE but then it will fall down with "ERR: Bootloader >> shutdown EFI x64 boot services!" implying that this is required. By >> having Xen expose in its header that its required it allows me to >> handle the situation gracefully in iPXE. >> >> To quote the multiboot2 spec exact: >> >> "This tag indicates that payload supports starting without terminating >> boot services." >> >> Unfortunately the spec is a bit vague and how I am reading it is: >> - no tag = exit boot services in the boot loader >> - tag present marked optional = boot loader can or cannot exit boot services >> - tag present marked required = boot loader cannot exit boot services > NACK, please take a look at section 3.1.4, Multiboot2 information request > in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the bootloader > than you think. The meaning of tag, if understood by Grub, is "don't exit boot services before passing control". The tag is currently marked as optional, which means Grub is free to ignore it if it doesn't understand it, resulting in EBS being called before passing control. Xen cannot cope with with EBS having been called, so must not be passed control under those circumstances. Doug's patch marks it as non-optional which, by that section above, requires Grub to fail with an error rather than proceeding, if it does not understand the tag. By my reading, Doug's patch looks correct. How does your interpretation of the spec differ? ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/boot: fix MB2 header to require EFI BS
On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote: > The EFI multiboot2 entry point currently requires EFI BootServices to > not have been exited however the header currently tells the boot > loader that Xen optionally supports EFI BootServices having been exited. > With this change Xen properly advertises that EFI must not be exited > allowing the boot loader to report an error that it cannot boot Xen if > it is unable to meet its needs. > > Signed-off-by: Doug Goldstein> --- > > This should likely be applied against Xen 4.9 and Xen 4.10 as well as > staging. I am trying to get multiboot2 support for iPXE and upstream > is concerned that leaving EFI BootServices enabled will not be > compatible with their aims to support Secure Boot. So when I build Hmmm... What are exact arguments for that? How do they implement e.g. chain loading then? What about the shim support? > my iPXE without support for passing on Boot Services, Xen will be > loaded by iPXE but then it will fall down with "ERR: Bootloader > shutdown EFI x64 boot services!" implying that this is required. By > having Xen expose in its header that its required it allows me to > handle the situation gracefully in iPXE. > > To quote the multiboot2 spec exact: > > "This tag indicates that payload supports starting without terminating > boot services." > > Unfortunately the spec is a bit vague and how I am reading it is: > - no tag = exit boot services in the boot loader > - tag present marked optional = boot loader can or cannot exit boot services > - tag present marked required = boot loader cannot exit boot services NACK, please take a look at section 3.1.4, Multiboot2 information request in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the bootloader than you think. > In the future I would like to add support to Xen to allow it to run > without boot services but presently that support isn't there. I tried that. This is difficult but not impossible. Hmmm... IIRC, some things are impossible. Please take a look at efi_multiboot2() and you quickly will know. Though why not try again. Daniel ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [linux-4.9 test] 115177: regressions - FAIL
flight 115177 linux-4.9 real [real] http://logs.test-lab.xenproject.org/osstest/logs/115177/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail REGR. vs. 114814 Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail in 115167 pass in 115177 test-armhf-armhf-xl-credit2 16 guest-start/debian.repeat fail pass in 115167 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 114814 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 114814 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 114814 test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qcow2 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-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass version targeted for testing: linux4d4a6a3f8a12602ce8dc800123715fe7b5c1c3a1 baseline version: linux5d7a76acad403638f635c918cc63d1d44ffa4065 Last test of basis 114814 2017-10-20 20:51:56 Z3 days Testing same since 114845 2017-10-21 16:14:17 Z3 days7 attempts People who touched revisions under test: Alex DeucherAlexandre Belloni Andrew Morton Anoob Soman Arnd Bergmann Bart Van Assche Ben Skeggs Bin Liu Borislav Petkov Christoph Lameter Christophe JAILLET Coly Li Dan Carpenter David Rientjes
[Xen-devel] [PATCH] x86/boot: fix MB2 header to require EFI BS
The EFI multiboot2 entry point currently requires EFI BootServices to not have been exited however the header currently tells the boot loader that Xen optionally supports EFI BootServices having been exited. With this change Xen properly advertises that EFI must not be exited allowing the boot loader to report an error that it cannot boot Xen if it is unable to meet its needs. Signed-off-by: Doug Goldstein--- This should likely be applied against Xen 4.9 and Xen 4.10 as well as staging. I am trying to get multiboot2 support for iPXE and upstream is concerned that leaving EFI BootServices enabled will not be compatible with their aims to support Secure Boot. So when I build my iPXE without support for passing on Boot Services, Xen will be loaded by iPXE but then it will fall down with "ERR: Bootloader shutdown EFI x64 boot services!" implying that this is required. By having Xen expose in its header that its required it allows me to handle the situation gracefully in iPXE. To quote the multiboot2 spec exact: "This tag indicates that payload supports starting without terminating boot services." Unfortunately the spec is a bit vague and how I am reading it is: - no tag = exit boot services in the boot loader - tag present marked optional = boot loader can or cannot exit boot services - tag present marked required = boot loader cannot exit boot services In the future I would like to add support to Xen to allow it to run without boot services but presently that support isn't there. --- xen/arch/x86/boot/head.S | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S index 9cc35da558..f76c2c0664 100644 --- a/xen/arch/x86/boot/head.S +++ b/xen/arch/x86/boot/head.S @@ -98,8 +98,8 @@ multiboot2_header_start: 0, /* Number of the lines - no preference. */ \ 0 /* Number of bits per pixel - no preference. */ -/* Request that ExitBootServices() not be called. */ -mb2ht_init MB2_HT(EFI_BS), MB2_HT(OPTIONAL) +/* Require that ExitBootServices() not be called. */ +mb2ht_init MB2_HT(EFI_BS), MB2_HT(REQUIRED) /* EFI64 Multiboot2 entry point. */ mb2ht_init MB2_HT(ENTRY_ADDRESS_EFI64), MB2_HT(OPTIONAL), \ -- 2.13.5 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [xen-unstable-smoke test] 115192: tolerable all pass - PUSHED
flight 115192 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/115192/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-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 version targeted for testing: xen be7f60b5a39741eab0a8fea0324f7be0cb724cfb baseline version: xen b8acf328ac86fbb45831917a61e94be2de34294d Last test of basis 115187 2017-10-24 15:02:14 Z0 days Testing same since 115192 2017-10-24 17:03:14 Z0 days1 attempts People who touched revisions under test: Jan Beulichjobs: build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-amd64-amd64-xl-qemuu-debianhvm-i386 pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : + branch=xen-unstable-smoke + revision=be7f60b5a39741eab0a8fea0324f7be0cb724cfb + . ./cri-lock-repos ++ . ./cri-common +++ . ./cri-getconfig export PERLLIB=.:. PERLLIB=.:. +++ umask 002 +++ getrepos getconfig Repos perl -e ' use Osstest; readglobalconfig(); print $c{"Repos"} or die $!; ' +++ local repos=/home/osstest/repos +++ '[' -z /home/osstest/repos ']' +++ '[' '!' -d /home/osstest/repos ']' +++ echo /home/osstest/repos ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x '!=' x/home/osstest/repos/lock ']' ++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock ++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push xen-unstable-smoke be7f60b5a39741eab0a8fea0324f7be0cb724cfb + branch=xen-unstable-smoke + revision=be7f60b5a39741eab0a8fea0324f7be0cb724cfb + . ./cri-lock-repos ++ . ./cri-common +++ . ./cri-getconfig export PERLLIB=.:.:. PERLLIB=.:.:. +++ umask 002 +++ getrepos getconfig Repos perl -e ' use Osstest; readglobalconfig(); print $c{"Repos"} or die $!; ' +++ local repos=/home/osstest/repos +++ '[' -z /home/osstest/repos ']' +++ '[' '!' -d /home/osstest/repos ']' +++ echo /home/osstest/repos ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']' + . ./cri-common ++ . ./cri-getconfig +++ export PERLLIB=.:.:.:. +++ PERLLIB=.:.:.:. ++ umask 002 + select_xenbranch + case "$branch" in + tree=xen + xenbranch=xen-unstable-smoke + qemuubranch=qemu-upstream-unstable + '[' xxen = xlinux ']' + linuxbranch= + '[' xqemu-upstream-unstable = x ']' + select_prevxenbranch ++ ./cri-getprevxenbranch xen-unstable-smoke + prevxenbranch=xen-4.9-testing + '[' xbe7f60b5a39741eab0a8fea0324f7be0cb724cfb = x ']' + : tested/2.6.39.x + . ./ap-common ++ : osst...@xenbits.xen.org +++ getconfig OsstestUpstream +++ perl -e ' use Osstest; readglobalconfig(); print $c{"OsstestUpstream"} or die $!; ' ++ : ++ : git://xenbits.xen.org/xen.git ++ : osst...@xenbits.xen.org:/home/xen/git/xen.git ++ : git://xenbits.xen.org/qemu-xen-traditional.git ++ : git://git.kernel.org ++ : git://git.kernel.org/pub/scm/linux/kernel/git ++ : git ++ : git://xenbits.xen.org/xtf.git ++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git ++ : git://xenbits.xen.org/xtf.git ++ : git://xenbits.xen.org/libvirt.git ++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git ++ : git://xenbits.xen.org/libvirt.git ++ : git://xenbits.xen.org/osstest/rumprun.git ++ : git ++ : git://xenbits.xen.org/osstest/rumprun.git ++ : osst...@xenbits.xen.org:/home/xen/git/osstest/rumprun.git ++ : git://git.seabios.org/seabios.git ++ : osst...@xenbits.xen.org:/home/xen/git/osstest/seabios.git ++ : git://xenbits.xen.org/osstest/seabios.git ++ : https://github.com/tianocore/edk2.git ++ : osst...@xenbits.xen.org:/home/xen/git/osstest/ovmf.git ++ : git://xenbits.xen.org/osstest/ovmf.git ++ :
Re: [Xen-devel] [PATCH 0/6] Intel Processor Trace virtulization enabling
On 21/10/17 21:02, Luwei Kang wrote: > Hi All, > > Here is a patch-series which adding Processor Trace enabling in XEN guest. > You can get It's software developer manuals from: > https://software.intel.com/sites/default/files/managed/c5/15/architecture-instruction-set-extensions-programming-reference.pdf > In Chapter 5 INTEL PROCESSOR TRACE: VMX IMPROVEMENTS. > > Introduction: > Intel Processor Trace (Intel PT) is an extension of Intel Architecture that > captures information about software execution using dedicated hardware > facilities that cause only minimal performance perturbation to the software > being traced. Details on the Intel PT infrastructure and trace capabilities > can be found in the Intel 64 and IA-32 Architectures Software Developer’s > Manual, Volume 3C. > > The suite of architecture changes serve to simplify the process of > virtualizing Intel PT for use by a guest software. There are two primary > elements to this new architecture support for VMX support improvements made > for Intel PT. > 1. Addition of a new guest IA32_RTIT_CTL value field to the VMCS. > — This serves to speed and simplify the process of disabling trace on VM > exit, and restoring it on VM entry. > 2. Enabling use of EPT to redirect PT output. > — This enables the VMM to elect to virtualize the PT output buffer using > EPT. In this mode, the CPU will treat PT output addresses as Guest Physical > Addresses (GPAs) and translate them using EPT. This means that Intel PT > output reads (of the ToPA table) and writes (of trace output) can cause EPT > violations, and other output events. Hello, Having read the new proposed extensions, I've got some architecture questions before diving into the patches themselves. First of all, is this technology expected to end up in Icelake, or something later? In Vol 3, the existing VMX support describes a number of scenarios (system wide, VMM-only, VM-only, guest aware), which require the use of MSR load lists to atomically alter the IA32_RTIT_* msrs. Obviously, system wide mode is incompatible with also allowing the guest to use PT itself, but what about Xen wanting to use PT for itself, and for the guest to use PT as well? Previously, this appears to be possible using the MSR load lists (albeit with Xen needing to shadow the ToPA records to cause the packet stream to end up in the right place). However, the new VM consistency checks require that using EPT redirection requires clear/load CTL on exit/entry be set, and having load on entry set requires the host TraceEn to be clear. Therefore, as far as I can see, allowing a guest to use PT via EPT now prohibits Xen also using PT for its own purposes outside of non-root mode. Is this intentional and/or expected, or have I misunderstood something in the manuals? Thanks, ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC 0/4] TEE mediator framework + OP-TEE mediator
Hi Julien, Jens, I'm looped in Jens Wiklander. He is one of OP-TEE maintainers. He also maintains TEE subsytem in Linux kernel. I CC'ed him in 4/4 patch, because only it concerned OP-TEE. But looks like discussion in this thread revolves primarily over OP-TEE, so I'm adding him there. Jens, if you want to catch up, you can find whole thread at [1]. On Tue, Oct 24, 2017 at 06:33:20PM +0100, Julien Grall wrote: > Hi, > > On 23/10/17 21:11, Volodymyr Babchuk wrote: > >On Mon, Oct 23, 2017 at 05:59:44PM +0100, Julien Grall wrote: > > > >>Hi Volodymyr, > >Hi Julien, > > > >>Let me begin the e-mail with I am not totally adversed to putting the TEE > >>mediator in Xen. At the moment, I am trying to understand the whole picture. > >Thanks for clarification. This is really reassuring :) > >In my turn, I'm not totally against TEE mediators in stubdoms. I'm only > >concerned about required efforts. > > > >>On 20/10/17 18:37, Volodymyr Babchuk wrote: > >>>On Fri, Oct 20, 2017 at 02:11:14PM +0100, Julien Grall wrote: > On 17/10/17 16:59, Volodymyr Babchuk wrote: > >On Mon, Oct 16, 2017 at 01:00:21PM +0100, Julien Grall wrote: > >>On 11/10/17 20:01, Volodymyr Babchuk wrote: > >>>I want to present TEE mediator, that was discussed earlier ([1]). > >>> > >>>I selected design with built-in mediators. This is easiest way, > >>>it removes many questions, it is easy to implement and maintain > >>>(at least I hope so). > >> > >>Well, it may close the technical questions but still leave the security > >>impact unanswered. I would have appreciated a summary of each approach > >>and > >>explain the pros/cons. > >This is the most secure way also. In terms of trust between guests and > >Xen at least. I'm worked with OP-TEE guys mostly, so when I hear about > >"security", my first thoughts are "Can TEE OS trust to XEN as a > >mediator? Can TEE client trust to XEN as a mediator?". And with > >current approach answer is "yes, they can, especially if XEN is a part > >of a chain of trust". > > > >But you probably wanted to ask "Can guest compromise whole system by > >using TEE mediator or TEE OS?". This is an interesting question. > >First let's discuss requirements for a TEE mediator. So, mediator > >should be able to: > > > > * Receive request to handle trapped SMC. This request should include > >user registers + some information about guest (at least domain id). > > * Pin/unpin domain memory pages. > > * Map domain memory pages into own address space with RW access. > > * Issue real SMC to a TEE. > > * Receive information about guest creation and destruction. > > * (Probably) inject IRQs into a domain (this can be not a requester > > domain, > >but some other domain, that also called to TEE). > > > >This is a minimal list of requirements. I think, this should be enough to > >implement mediator for OP-TEE. But I can't say for sure for other TEEs. > > > >Let's consider possible approaches: > > > >1. Mediator right in XEN, works at EL2. > >Pros: > > * Mediator can use all XEN APIs > > * As mediator resides in XEN, it can be checked together with XEN > > for a validity (trusted boot). > > * Mediator is initialized before Dom0. Dom0 can work with a TEE. > > * No extra context switches, no special ABI between XEN and > > mediator. > > > >Cons: > > * Because it lives in EL2, it can compromise whole hypervisor, > > if there is a security bug in mediator code. > > * No support for closed source TEEs. > > Another cons is you assume TEE API is fully stable and will not change. > Imagine a new function is added, or a vendor decided to hence with a new > set > of API. How will you know Xen is safe to use it? > >>>With whitelisting, as you correctly suggested below. XEN will process > >>>only know requests. Anything that looks unfimiliar should be rejected. > >> > >>Let's imagine the guest is running on a platform with a newer version of > >>TEE. This guest will probe the version of OP-TEE and knows the new function > >>is present. > >This request will be handled mediator. At this moment, OP-TEE client does > >not use versions. Instead it uses capability flags. So, mediator should > >filter all unknown caps. This will force guest to use only supported > >subset of features. > > One more question. Does it mean new functions will never be added in current > capabilities? AFAIK, now. That would break backward compatibility. > >If, in the future, client will relly on versions (i.e. due to dramatic > >protocol change), mediator can either downgrade version or refuse to work > >at all. > > Makes sense. > > > > >>If as you said Xen is using a whitelist, this means the hypervisor will > >>return unimplemented. > >>How do you expect the guest to behave in
[Xen-devel] [xen-unstable test] 115174: regressions - FAIL
flight 115174 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/115174/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stopfail REGR. vs. 114644 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail REGR. vs. 114644 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail REGR. vs. 114644 Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-qemuu-ovmf-amd64 14 guest-localmigrate fail in 115163 pass in 115174 test-armhf-armhf-xl-credit2 16 guest-start/debian.repeat fail pass in 115163 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail like 114644 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 114644 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 114644 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 114644 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 114644 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 114644 test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qcow2 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass version targeted for testing: xen 1f2c7894dfe3d52f33655de202bd474999a1637b baseline version: xen 24fb44e971a62b345c7b6ca3c03b454a1e150abe Last test of basis 114644 2017-10-17 10:49:11 Z7 days Failing since114670 2017-10-18 05:03:38 Z6 days 10 attempts Testing same since 115163 2017-10-23 21:47:10 Z0 days2 attempts People who touched revisions under test: Andrew CooperAnthony PERARD David Esler George Dunlap Ian Jackson Jan Beulich Julien Grall Roger Pau Monné Stefano Stabellini Tim Deegan Wei Liu jobs: build-amd64-xsm
Re: [Xen-devel] [PATCH v5.1 7/8] os-posix: Provide new -runas : facility
Anthony PERARD writes ("Re: [PATCH v5.1 7/8] os-posix: Provide new -runas : facility"): > On Fri, Oct 20, 2017 at 02:38:21PM +0100, Ian Jackson wrote: > > +static bool os_parse_runas_uid_gid(const char *optarg) ... > > +errno = 0; > > +lv = strtoul(optarg, , 0); /* can't qemu_strtoul, want *ep==':' */ > > Should strtoul base be 10? If that matter. If someone wants to write uids in hex then I don't see a reason to stop them... > > -if (!user_pwd) { > > -fprintf(stderr, "User \"%s\" doesn't exist\n", optarg); > > +if (!user_pwd && !os_parse_runas_uid_gid(optarg)) { > > +fprintf(stderr, > > +"User \"%s\" doesn't exist (and is not .)\n", > > The error message have not been update, I think it should be : Oops. > With the error message fix: > Reviewed-by: Anthony PERARDThanks, Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC 05/14] xen: vmx: Disable the 2M/1G superpage when SPP enabled
On Fri, Oct 20, 2017 at 2:44 AM, Yi Zhangwrote: > On 2017-10-19 at 12:17:12 -0600, Tamas K Lengyel wrote: >> On Thu, Oct 19, 2017 at 2:11 AM, Zhang Yi wrote: >> > From: Zhang Yi Z >> > >> > Current we only support Sub-page Protection on the 4k >> > page table. >> > >> > Signed-off-by: Zhang Yi Z >> > --- >> > xen/arch/x86/hvm/vmx/vmx.c | 6 ++ >> > 1 file changed, 6 insertions(+) >> > >> > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c >> > index 04ae0d6..a4c24bb 100644 >> > --- a/xen/arch/x86/hvm/vmx/vmx.c >> > +++ b/xen/arch/x86/hvm/vmx/vmx.c >> > @@ -2497,6 +2497,12 @@ const struct hvm_function_table * __init >> > start_vmx(void) >> > vmx_function_table.get_guest_bndcfgs = vmx_get_guest_bndcfgs; >> > } >> > >> > +if ( cpu_has_vmx_ept_spp ) >> >> I think this really only ought to happen if the command-line option >> has also been enabled. > > Sorry, didn't catch your point, the command line option opt_hap_2m and > opt_hap_1G was enable by default, I need to disable the supper page > when spp feature enabled. Did you mean that if we enable 2M/1G by > command-line we couldn't disable it here? yes, it is, I will improve > this logic. Thank you Tamas. I meant that right now "cpu_has_vmx_ept_spp" looks like just checks whether the CPU supports SPP, not whether the command-line option was set to enable it. If the command line option is not set (or specifically disables SPP) then the large pages shouldn't get disabled. > >> >> > +{ >> > +vmx_function_table.hap_capabilities &= ~HVM_HAP_SUPERPAGE_2MB; >> > +vmx_function_table.hap_capabilities &= ~HVM_HAP_SUPERPAGE_1GB; >> > +} >> > + >> > setup_vmcs_dump(); >> > >> > lbr_tsx_fixup_check(); >> > -- >> > 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6 11/13] xen/pvcalls: implement poll command
For active sockets, check the indexes and use the inflight_conn_req waitqueue to wait. For passive sockets if an accept is outstanding (PVCALLS_FLAG_ACCEPT_INFLIGHT), check if it has been answered by looking at bedata->rsp[req_id]. If so, return POLLIN. Otherwise use the inflight_accept_req waitqueue. If no accepts are inflight, send PVCALLS_POLL to the backend. If we have outstanding POLL requests awaiting for a response use the inflight_req waitqueue: inflight_req is awaken when a new response is received; on wakeup we check whether the POLL response is arrived by looking at the PVCALLS_FLAG_POLL_RET flag. We set the flag from pvcalls_front_event_handler, if the response was for a POLL command. In pvcalls_front_event_handler, get the struct sock_mapping from the poll id (we previously converted struct sock_mapping* to uint64_t and used it as id). Signed-off-by: Stefano StabelliniReviewed-by: Boris Ostrovsky CC: boris.ostrov...@oracle.com CC: jgr...@suse.com --- drivers/xen/pvcalls-front.c | 144 +--- drivers/xen/pvcalls-front.h | 3 + 2 files changed, 138 insertions(+), 9 deletions(-) diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c index a07be6a..4a413ff 100644 --- a/drivers/xen/pvcalls-front.c +++ b/drivers/xen/pvcalls-front.c @@ -83,6 +83,8 @@ struct sock_mapping { * Only one poll operation can be inflight for a given socket. */ #define PVCALLS_FLAG_ACCEPT_INFLIGHT 0 +#define PVCALLS_FLAG_POLL_INFLIGHT 1 +#define PVCALLS_FLAG_POLL_RET2 uint8_t flags; uint32_t inflight_req_id; struct sock_mapping *accept_map; @@ -154,15 +156,32 @@ static irqreturn_t pvcalls_front_event_handler(int irq, void *dev_id) rsp = RING_GET_RESPONSE(>ring, bedata->ring.rsp_cons); req_id = rsp->req_id; - dst = (uint8_t *)>rsp[req_id] + sizeof(rsp->req_id); - src = (uint8_t *)rsp + sizeof(rsp->req_id); - memcpy(dst, src, sizeof(*rsp) - sizeof(rsp->req_id)); - /* -* First copy the rest of the data, then req_id. It is -* paired with the barrier when accessing bedata->rsp. -*/ - smp_wmb(); - bedata->rsp[req_id].req_id = rsp->req_id; + if (rsp->cmd == PVCALLS_POLL) { + struct sock_mapping *map = (struct sock_mapping *) + rsp->u.poll.id; + + clear_bit(PVCALLS_FLAG_POLL_INFLIGHT, + (void *)>passive.flags); + /* +* clear INFLIGHT, then set RET. It pairs with +* the checks at the beginning of +* pvcalls_front_poll_passive. +*/ + smp_wmb(); + set_bit(PVCALLS_FLAG_POLL_RET, + (void *)>passive.flags); + } else { + dst = (uint8_t *)>rsp[req_id] + + sizeof(rsp->req_id); + src = (uint8_t *)rsp + sizeof(rsp->req_id); + memcpy(dst, src, sizeof(*rsp) - sizeof(rsp->req_id)); + /* +* First copy the rest of the data, then req_id. It is +* paired with the barrier when accessing bedata->rsp. +*/ + smp_wmb(); + bedata->rsp[req_id].req_id = req_id; + } done = 1; bedata->ring.rsp_cons++; @@ -840,6 +859,113 @@ int pvcalls_front_accept(struct socket *sock, struct socket *newsock, int flags) return ret; } +static unsigned int pvcalls_front_poll_passive(struct file *file, + struct pvcalls_bedata *bedata, + struct sock_mapping *map, + poll_table *wait) +{ + int notify, req_id, ret; + struct xen_pvcalls_request *req; + + if (test_bit(PVCALLS_FLAG_ACCEPT_INFLIGHT, +(void *)>passive.flags)) { + uint32_t req_id = READ_ONCE(map->passive.inflight_req_id); + + if (req_id != PVCALLS_INVALID_ID && + READ_ONCE(bedata->rsp[req_id].req_id) == req_id) + return POLLIN | POLLRDNORM; + + poll_wait(file, >passive.inflight_accept_req, wait); + return 0; + } + + if (test_and_clear_bit(PVCALLS_FLAG_POLL_RET, + (void *)>passive.flags)) + return POLLIN | POLLRDNORM; + + /* +* First check RET, then INFLIGHT. No barriers
[Xen-devel] [PATCH v6 02/13] xen/pvcalls: implement frontend disconnect
Introduce a data structure named pvcalls_bedata. It contains pointers to the command ring, the event channel, a list of active sockets and a list of passive sockets. Lists accesses are protected by a spin_lock. Introduce a waitqueue to allow waiting for a response on commands sent to the backend. Introduce an array of struct xen_pvcalls_response to store commands responses. pvcalls_refcount is used to keep count of the outstanding pvcalls users. Only remove connections once the refcount is zero. Implement pvcalls frontend removal function. Go through the list of active and passive sockets and free them all, one at a time. Signed-off-by: Stefano StabelliniCC: boris.ostrov...@oracle.com CC: jgr...@suse.com --- drivers/xen/pvcalls-front.c | 65 + 1 file changed, 65 insertions(+) diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c index a8d38c2..4babacf 100644 --- a/drivers/xen/pvcalls-front.c +++ b/drivers/xen/pvcalls-front.c @@ -20,6 +20,45 @@ #include #include +#define PVCALLS_INVALID_ID UINT_MAX +#define PVCALLS_RING_ORDER XENBUS_MAX_RING_GRANT_ORDER +#define PVCALLS_NR_RSP_PER_RING __CONST_RING_SIZE(xen_pvcalls, XEN_PAGE_SIZE) + +struct pvcalls_bedata { + struct xen_pvcalls_front_ring ring; + grant_ref_t ref; + int irq; + + struct list_head socket_mappings; + spinlock_t socket_lock; + + wait_queue_head_t inflight_req; + struct xen_pvcalls_response rsp[PVCALLS_NR_RSP_PER_RING]; +}; +/* Only one front/back connection supported. */ +static struct xenbus_device *pvcalls_front_dev; +static atomic_t pvcalls_refcount; + +/* first increment refcount, then proceed */ +#define pvcalls_enter() { \ + atomic_inc(_refcount); \ +} + +/* first complete other operations, then decrement refcount */ +#define pvcalls_exit() {\ + atomic_dec(_refcount); \ +} + +static irqreturn_t pvcalls_front_event_handler(int irq, void *dev_id) +{ + return IRQ_HANDLED; +} + +static void pvcalls_front_free_map(struct pvcalls_bedata *bedata, + struct sock_mapping *map, bool locked) +{ +} + static const struct xenbus_device_id pvcalls_front_ids[] = { { "pvcalls" }, { "" } @@ -27,6 +66,32 @@ static int pvcalls_front_remove(struct xenbus_device *dev) { + struct pvcalls_bedata *bedata; + struct sock_mapping *map = NULL, *n; + + bedata = dev_get_drvdata(_front_dev->dev); + dev_set_drvdata(>dev, NULL); + pvcalls_front_dev = NULL; + if (bedata->irq >= 0) + unbind_from_irqhandler(bedata->irq, dev); + + smp_mb(); + while (atomic_read(_refcount) > 0) + cpu_relax(); + list_for_each_entry_safe(map, n, >socket_mappings, list) { + if (map->active_socket) { + /* No need to lock, refcount is 0 */ + pvcalls_front_free_map(bedata, map, true); + } else { + list_del(>list); + kfree(map); + } + } + if (bedata->ref >= 0) + gnttab_end_foreign_access(bedata->ref, 0, 0); + kfree(bedata->ring.sring); + kfree(bedata); + xenbus_switch_state(dev, XenbusStateClosed); return 0; } -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6 07/13] xen/pvcalls: implement listen command
Send PVCALLS_LISTEN to the backend. Signed-off-by: Stefano StabelliniReviewed-by: Boris Ostrovsky CC: boris.ostrov...@oracle.com CC: jgr...@suse.com --- drivers/xen/pvcalls-front.c | 57 + drivers/xen/pvcalls-front.h | 1 + 2 files changed, 58 insertions(+) diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c index 81baebd..f616378 100644 --- a/drivers/xen/pvcalls-front.c +++ b/drivers/xen/pvcalls-front.c @@ -412,6 +412,63 @@ int pvcalls_front_bind(struct socket *sock, struct sockaddr *addr, int addr_len) return 0; } +int pvcalls_front_listen(struct socket *sock, int backlog) +{ + struct pvcalls_bedata *bedata; + struct sock_mapping *map; + struct xen_pvcalls_request *req; + int notify, req_id, ret; + + pvcalls_enter(); + if (!pvcalls_front_dev) { + pvcalls_exit(); + return -ENOTCONN; + } + bedata = dev_get_drvdata(_front_dev->dev); + + map = (struct sock_mapping *) sock->sk->sk_send_head; + if (!map) { + pvcalls_exit(); + return -ENOTSOCK; + } + + if (map->passive.status != PVCALLS_STATUS_BIND) { + pvcalls_exit(); + return -EOPNOTSUPP; + } + + spin_lock(>socket_lock); + ret = get_request(bedata, _id); + if (ret < 0) { + spin_unlock(>socket_lock); + pvcalls_exit(); + return ret; + } + req = RING_GET_REQUEST(>ring, req_id); + req->req_id = req_id; + req->cmd = PVCALLS_LISTEN; + req->u.listen.id = (uint64_t) map; + req->u.listen.backlog = backlog; + + bedata->ring.req_prod_pvt++; + RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(>ring, notify); + spin_unlock(>socket_lock); + if (notify) + notify_remote_via_irq(bedata->irq); + + wait_event(bedata->inflight_req, + READ_ONCE(bedata->rsp[req_id].req_id) == req_id); + + /* read req_id, then the content */ + smp_rmb(); + ret = bedata->rsp[req_id].ret; + bedata->rsp[req_id].req_id = PVCALLS_INVALID_ID; + + map->passive.status = PVCALLS_STATUS_LISTEN; + pvcalls_exit(); + return ret; +} + static const struct xenbus_device_id pvcalls_front_ids[] = { { "pvcalls" }, { "" } diff --git a/drivers/xen/pvcalls-front.h b/drivers/xen/pvcalls-front.h index 8b0a274..aa8fe10 100644 --- a/drivers/xen/pvcalls-front.h +++ b/drivers/xen/pvcalls-front.h @@ -9,5 +9,6 @@ int pvcalls_front_connect(struct socket *sock, struct sockaddr *addr, int pvcalls_front_bind(struct socket *sock, struct sockaddr *addr, int addr_len); +int pvcalls_front_listen(struct socket *sock, int backlog); #endif -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6 06/13] xen/pvcalls: implement bind command
Send PVCALLS_BIND to the backend. Introduce a new structure, part of struct sock_mapping, to store information specific to passive sockets. Introduce a status field to keep track of the status of the passive socket. Signed-off-by: Stefano StabelliniReviewed-by: Boris Ostrovsky CC: boris.ostrov...@oracle.com CC: jgr...@suse.com --- drivers/xen/pvcalls-front.c | 66 + drivers/xen/pvcalls-front.h | 3 +++ 2 files changed, 69 insertions(+) diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c index c426d41..81baebd 100644 --- a/drivers/xen/pvcalls-front.c +++ b/drivers/xen/pvcalls-front.c @@ -70,6 +70,13 @@ struct sock_mapping { wait_queue_head_t inflight_conn_req; } active; + struct { + /* Socket status */ +#define PVCALLS_STATUS_UNINITALIZED 0 +#define PVCALLS_STATUS_BIND 1 +#define PVCALLS_STATUS_LISTEN2 + uint8_t status; + } passive; }; }; @@ -346,6 +353,65 @@ int pvcalls_front_connect(struct socket *sock, struct sockaddr *addr, return ret; } +int pvcalls_front_bind(struct socket *sock, struct sockaddr *addr, int addr_len) +{ + struct pvcalls_bedata *bedata; + struct sock_mapping *map = NULL; + struct xen_pvcalls_request *req; + int notify, req_id, ret; + + if (addr->sa_family != AF_INET || sock->type != SOCK_STREAM) + return -EOPNOTSUPP; + + pvcalls_enter(); + if (!pvcalls_front_dev) { + pvcalls_exit(); + return -ENOTCONN; + } + bedata = dev_get_drvdata(_front_dev->dev); + + map = (struct sock_mapping *) sock->sk->sk_send_head; + if (map == NULL) { + pvcalls_exit(); + return -ENOTSOCK; + } + + spin_lock(>socket_lock); + ret = get_request(bedata, _id); + if (ret < 0) { + spin_unlock(>socket_lock); + pvcalls_exit(); + return ret; + } + req = RING_GET_REQUEST(>ring, req_id); + req->req_id = req_id; + map->sock = sock; + req->cmd = PVCALLS_BIND; + req->u.bind.id = (uint64_t)map; + memcpy(req->u.bind.addr, addr, sizeof(*addr)); + req->u.bind.len = addr_len; + + map->active_socket = false; + + bedata->ring.req_prod_pvt++; + RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(>ring, notify); + spin_unlock(>socket_lock); + if (notify) + notify_remote_via_irq(bedata->irq); + + wait_event(bedata->inflight_req, + READ_ONCE(bedata->rsp[req_id].req_id) == req_id); + + /* read req_id, then the content */ + smp_rmb(); + ret = bedata->rsp[req_id].ret; + bedata->rsp[req_id].req_id = PVCALLS_INVALID_ID; + + map->passive.status = PVCALLS_STATUS_BIND; + pvcalls_exit(); + return 0; +} + static const struct xenbus_device_id pvcalls_front_ids[] = { { "pvcalls" }, { "" } diff --git a/drivers/xen/pvcalls-front.h b/drivers/xen/pvcalls-front.h index 63b0417..8b0a274 100644 --- a/drivers/xen/pvcalls-front.h +++ b/drivers/xen/pvcalls-front.h @@ -6,5 +6,8 @@ int pvcalls_front_socket(struct socket *sock); int pvcalls_front_connect(struct socket *sock, struct sockaddr *addr, int addr_len, int flags); +int pvcalls_front_bind(struct socket *sock, + struct sockaddr *addr, + int addr_len); #endif -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6 13/13] xen: introduce a Kconfig option to enable the pvcalls frontend
Also add pvcalls-front to the Makefile. Signed-off-by: Stefano StabelliniCC: boris.ostrov...@oracle.com CC: jgr...@suse.com --- drivers/xen/Kconfig | 9 + drivers/xen/Makefile | 1 + 2 files changed, 10 insertions(+) diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig index 4545561..0b2c828 100644 --- a/drivers/xen/Kconfig +++ b/drivers/xen/Kconfig @@ -196,6 +196,15 @@ config XEN_PCIDEV_BACKEND If in doubt, say m. +config XEN_PVCALLS_FRONTEND + tristate "XEN PV Calls frontend driver" + depends on INET && XEN + help + Experimental frontend for the Xen PV Calls protocol + (https://xenbits.xen.org/docs/unstable/misc/pvcalls.html). It + sends a small set of POSIX calls to the backend, which + implements them. + config XEN_PVCALLS_BACKEND bool "XEN PV Calls backend driver" depends on INET && XEN && XEN_BACKEND diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile index 480b928..afb9e03 100644 --- a/drivers/xen/Makefile +++ b/drivers/xen/Makefile @@ -39,6 +39,7 @@ obj-$(CONFIG_XEN_EFI) += efi.o obj-$(CONFIG_XEN_SCSI_BACKEND) += xen-scsiback.o obj-$(CONFIG_XEN_AUTO_XLATE) += xlate_mmu.o obj-$(CONFIG_XEN_PVCALLS_BACKEND) += pvcalls-back.o +obj-$(CONFIG_XEN_PVCALLS_FRONTEND) += pvcalls-front.o xen-evtchn-y := evtchn.o xen-gntdev-y := gntdev.o xen-gntalloc-y := gntalloc.o -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6 01/13] xen/pvcalls: introduce the pvcalls xenbus frontend
Introduce a xenbus frontend for the pvcalls protocol, as defined by https://xenbits.xen.org/docs/unstable/misc/pvcalls.html. This patch only adds the stubs, the code will be added by the following patches. Signed-off-by: Stefano StabelliniReviewed-by: Boris Ostrovsky CC: boris.ostrov...@oracle.com CC: jgr...@suse.com --- drivers/xen/pvcalls-front.c | 61 + 1 file changed, 61 insertions(+) create mode 100644 drivers/xen/pvcalls-front.c diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c new file mode 100644 index 000..a8d38c2 --- /dev/null +++ b/drivers/xen/pvcalls-front.c @@ -0,0 +1,61 @@ +/* + * (c) 2017 Stefano Stabellini + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * 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. + */ + +#include + +#include +#include +#include +#include +#include + +static const struct xenbus_device_id pvcalls_front_ids[] = { + { "pvcalls" }, + { "" } +}; + +static int pvcalls_front_remove(struct xenbus_device *dev) +{ + return 0; +} + +static int pvcalls_front_probe(struct xenbus_device *dev, + const struct xenbus_device_id *id) +{ + return 0; +} + +static void pvcalls_front_changed(struct xenbus_device *dev, + enum xenbus_state backend_state) +{ +} + +static struct xenbus_driver pvcalls_front_driver = { + .ids = pvcalls_front_ids, + .probe = pvcalls_front_probe, + .remove = pvcalls_front_remove, + .otherend_changed = pvcalls_front_changed, +}; + +static int __init pvcalls_frontend_init(void) +{ + if (!xen_domain()) + return -ENODEV; + + pr_info("Initialising Xen pvcalls frontend driver\n"); + + return xenbus_register_frontend(_front_driver); +} + +module_init(pvcalls_frontend_init); -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6 03/13] xen/pvcalls: connect to the backend
Implement the probe function for the pvcalls frontend. Read the supported versions, max-page-order and function-calls nodes from xenstore. Only one frontend<->backend connection is supported at any given time for a guest. Store the active frontend device to a static pointer. Introduce a stub functions for the event handler. Signed-off-by: Stefano StabelliniReviewed-by: Boris Ostrovsky CC: boris.ostrov...@oracle.com CC: jgr...@suse.com --- drivers/xen/pvcalls-front.c | 132 1 file changed, 132 insertions(+) diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c index 4babacf..530ef05 100644 --- a/drivers/xen/pvcalls-front.c +++ b/drivers/xen/pvcalls-front.c @@ -98,12 +98,144 @@ static int pvcalls_front_remove(struct xenbus_device *dev) static int pvcalls_front_probe(struct xenbus_device *dev, const struct xenbus_device_id *id) { + int ret = -ENOMEM, evtchn, i; + unsigned int max_page_order, function_calls, len; + char *versions; + grant_ref_t gref_head = 0; + struct xenbus_transaction xbt; + struct pvcalls_bedata *bedata = NULL; + struct xen_pvcalls_sring *sring; + + if (pvcalls_front_dev != NULL) { + dev_err(>dev, "only one PV Calls connection supported\n"); + return -EINVAL; + } + + versions = xenbus_read(XBT_NIL, dev->otherend, "versions", ); + if (!len) + return -EINVAL; + if (strcmp(versions, "1")) { + kfree(versions); + return -EINVAL; + } + kfree(versions); + max_page_order = xenbus_read_unsigned(dev->otherend, + "max-page-order", 0); + if (max_page_order < PVCALLS_RING_ORDER) + return -ENODEV; + function_calls = xenbus_read_unsigned(dev->otherend, + "function-calls", 0); + /* See XENBUS_FUNCTIONS_CALLS in pvcalls.h */ + if (function_calls != 1) + return -ENODEV; + pr_info("%s max-page-order is %u\n", __func__, max_page_order); + + bedata = kzalloc(sizeof(struct pvcalls_bedata), GFP_KERNEL); + if (!bedata) + return -ENOMEM; + + dev_set_drvdata(>dev, bedata); + pvcalls_front_dev = dev; + init_waitqueue_head(>inflight_req); + INIT_LIST_HEAD(>socket_mappings); + spin_lock_init(>socket_lock); + bedata->irq = -1; + bedata->ref = -1; + + for (i = 0; i < PVCALLS_NR_RSP_PER_RING; i++) + bedata->rsp[i].req_id = PVCALLS_INVALID_ID; + + sring = (struct xen_pvcalls_sring *) __get_free_page(GFP_KERNEL | +__GFP_ZERO); + if (!sring) + goto error; + SHARED_RING_INIT(sring); + FRONT_RING_INIT(>ring, sring, XEN_PAGE_SIZE); + + ret = xenbus_alloc_evtchn(dev, ); + if (ret) + goto error; + + bedata->irq = bind_evtchn_to_irqhandler(evtchn, + pvcalls_front_event_handler, + 0, "pvcalls-frontend", dev); + if (bedata->irq < 0) { + ret = bedata->irq; + goto error; + } + + ret = gnttab_alloc_grant_references(1, _head); + if (ret < 0) + goto error; + bedata->ref = gnttab_claim_grant_reference(_head); + if (bedata->ref < 0) { + ret = bedata->ref; + goto error; + } + gnttab_grant_foreign_access_ref(bedata->ref, dev->otherend_id, + virt_to_gfn((void *)sring), 0); + + again: + ret = xenbus_transaction_start(); + if (ret) { + xenbus_dev_fatal(dev, ret, "starting transaction"); + goto error; + } + ret = xenbus_printf(xbt, dev->nodename, "version", "%u", 1); + if (ret) + goto error_xenbus; + ret = xenbus_printf(xbt, dev->nodename, "ring-ref", "%d", bedata->ref); + if (ret) + goto error_xenbus; + ret = xenbus_printf(xbt, dev->nodename, "port", "%u", + evtchn); + if (ret) + goto error_xenbus; + ret = xenbus_transaction_end(xbt, 0); + if (ret) { + if (ret == -EAGAIN) + goto again; + xenbus_dev_fatal(dev, ret, "completing transaction"); + goto error; + } + xenbus_switch_state(dev, XenbusStateInitialised); + return 0; + + error_xenbus: + xenbus_transaction_end(xbt, 1); + xenbus_dev_fatal(dev, ret, "writing xenstore"); + error: + pvcalls_front_remove(dev); + return ret; } static void pvcalls_front_changed(struct xenbus_device *dev, enum
[Xen-devel] [PATCH v6 12/13] xen/pvcalls: implement release command
Send PVCALLS_RELEASE to the backend and wait for a reply. Take both in_mutex and out_mutex to avoid concurrent accesses. Then, free the socket. For passive sockets, check whether we have already pre-allocated an active socket for the purpose of being accepted. If so, free that as well. Signed-off-by: Stefano StabelliniCC: boris.ostrov...@oracle.com CC: jgr...@suse.com --- drivers/xen/pvcalls-front.c | 100 drivers/xen/pvcalls-front.h | 1 + 2 files changed, 101 insertions(+) diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c index 4a413ff..7abc039 100644 --- a/drivers/xen/pvcalls-front.c +++ b/drivers/xen/pvcalls-front.c @@ -199,6 +199,23 @@ static irqreturn_t pvcalls_front_event_handler(int irq, void *dev_id) static void pvcalls_front_free_map(struct pvcalls_bedata *bedata, struct sock_mapping *map, bool locked) { + int i; + + unbind_from_irqhandler(map->active.irq, map); + + if (!locked) + spin_lock(>socket_lock); + if (!list_empty(>list)) + list_del_init(>list); + if (!locked) + spin_unlock(>socket_lock); + + for (i = 0; i < (1 << PVCALLS_RING_ORDER); i++) + gnttab_end_foreign_access(map->active.ring->ref[i], 0, 0); + gnttab_end_foreign_access(map->active.ref, 0, 0); + free_page((unsigned long)map->active.ring); + + kfree(map); } static irqreturn_t pvcalls_front_conn_handler(int irq, void *sock_map) @@ -966,6 +983,89 @@ unsigned int pvcalls_front_poll(struct file *file, struct socket *sock, return ret; } +int pvcalls_front_release(struct socket *sock) +{ + struct pvcalls_bedata *bedata; + struct sock_mapping *map; + int req_id, notify, ret; + struct xen_pvcalls_request *req; + + if (sock->sk == NULL) + return 0; + + pvcalls_enter(); + if (!pvcalls_front_dev) { + pvcalls_exit(); + return -EIO; + } + + bedata = dev_get_drvdata(_front_dev->dev); + + map = (struct sock_mapping *) sock->sk->sk_send_head; + if (map == NULL) { + pvcalls_exit(); + return 0; + } + + spin_lock(>socket_lock); + ret = get_request(bedata, _id); + if (ret < 0) { + spin_unlock(>socket_lock); + pvcalls_exit(); + return ret; + } + sock->sk->sk_send_head = NULL; + + req = RING_GET_REQUEST(>ring, req_id); + req->req_id = req_id; + req->cmd = PVCALLS_RELEASE; + req->u.release.id = (uint64_t)map; + + bedata->ring.req_prod_pvt++; + RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(>ring, notify); + spin_unlock(>socket_lock); + if (notify) + notify_remote_via_irq(bedata->irq); + + wait_event(bedata->inflight_req, + READ_ONCE(bedata->rsp[req_id].req_id) == req_id); + + if (map->active_socket) { + /* +* Set in_error and wake up inflight_conn_req to force +* recvmsg waiters to exit. +*/ + map->active.ring->in_error = -EBADF; + wake_up_interruptible(>active.inflight_conn_req); + + /* +* Wait until there are no more waiters on the mutexes. +* We know that no new waiters can be added because sk_send_head +* is set to NULL -- we only need to wait for the existing +* waiters to return. +*/ + while (!mutex_trylock(>active.in_mutex) || + !mutex_trylock(>active.out_mutex)) + cpu_relax(); + + pvcalls_front_free_map(bedata, map, false); + } else { + spin_lock(>socket_lock); + if (READ_ONCE(map->passive.inflight_req_id) != + PVCALLS_INVALID_ID) { + pvcalls_front_free_map(bedata, + map->passive.accept_map, true); + } + list_del(>list); + kfree(map); + spin_unlock(>socket_lock); + } + WRITE_ONCE(bedata->rsp[req_id].req_id, PVCALLS_INVALID_ID); + + pvcalls_exit(); + return 0; +} + static const struct xenbus_device_id pvcalls_front_ids[] = { { "pvcalls" }, { "" } diff --git a/drivers/xen/pvcalls-front.h b/drivers/xen/pvcalls-front.h index 25e05b8..3332978 100644 --- a/drivers/xen/pvcalls-front.h +++ b/drivers/xen/pvcalls-front.h @@ -23,5 +23,6 @@ int pvcalls_front_recvmsg(struct socket *sock, unsigned int pvcalls_front_poll(struct file *file, struct socket *sock, poll_table *wait); +int pvcalls_front_release(struct socket *sock); #endif -- 1.9.1
[Xen-devel] [PATCH v6 10/13] xen/pvcalls: implement recvmsg
Implement recvmsg by copying data from the "in" ring. If not enough data is available and the recvmsg call is blocking, then wait on the inflight_conn_req waitqueue. Take the active socket in_mutex so that only one function can access the ring at any given time. Signed-off-by: Stefano StabelliniReviewed-by: Boris Ostrovsky CC: boris.ostrov...@oracle.com CC: jgr...@suse.com --- drivers/xen/pvcalls-front.c | 108 drivers/xen/pvcalls-front.h | 4 ++ 2 files changed, 112 insertions(+) diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c index 27b7970..a07be6a 100644 --- a/drivers/xen/pvcalls-front.c +++ b/drivers/xen/pvcalls-front.c @@ -117,6 +117,20 @@ static bool pvcalls_front_write_todo(struct sock_mapping *map) return !!(size - pvcalls_queued(prod, cons, size)); } +static bool pvcalls_front_read_todo(struct sock_mapping *map) +{ + struct pvcalls_data_intf *intf = map->active.ring; + RING_IDX cons, prod; + int32_t error; + + cons = intf->in_cons; + prod = intf->in_prod; + error = intf->in_error; + return (error != 0 || + pvcalls_queued(prod, cons, + XEN_FLEX_RING_SIZE(PVCALLS_RING_ORDER)) != 0); +} + static irqreturn_t pvcalls_front_event_handler(int irq, void *dev_id) { struct xenbus_device *dev = dev_id; @@ -481,6 +495,100 @@ int pvcalls_front_sendmsg(struct socket *sock, struct msghdr *msg, return tot_sent; } +static int __read_ring(struct pvcalls_data_intf *intf, + struct pvcalls_data *data, + struct iov_iter *msg_iter, + size_t len, int flags) +{ + RING_IDX cons, prod, size, masked_prod, masked_cons; + RING_IDX array_size = XEN_FLEX_RING_SIZE(PVCALLS_RING_ORDER); + int32_t error; + + cons = intf->in_cons; + prod = intf->in_prod; + error = intf->in_error; + /* get pointers before reading from the ring */ + virt_rmb(); + if (error < 0) + return error; + + size = pvcalls_queued(prod, cons, array_size); + masked_prod = pvcalls_mask(prod, array_size); + masked_cons = pvcalls_mask(cons, array_size); + + if (size == 0) + return 0; + + if (len > size) + len = size; + + if (masked_prod > masked_cons) { + copy_to_iter(data->in + masked_cons, len, msg_iter); + } else { + if (len > (array_size - masked_cons)) { + copy_to_iter(data->in + masked_cons, +array_size - masked_cons, msg_iter); + copy_to_iter(data->in, +len - (array_size - masked_cons), +msg_iter); + } else { + copy_to_iter(data->in + masked_cons, len, msg_iter); + } + } + /* read data from the ring before increasing the index */ + virt_mb(); + if (!(flags & MSG_PEEK)) + intf->in_cons += len; + + return len; +} + +int pvcalls_front_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, +int flags) +{ + struct pvcalls_bedata *bedata; + int ret; + struct sock_mapping *map; + + if (flags & (MSG_CMSG_CLOEXEC|MSG_ERRQUEUE|MSG_OOB|MSG_TRUNC)) + return -EOPNOTSUPP; + + pvcalls_enter(); + if (!pvcalls_front_dev) { + pvcalls_exit(); + return -ENOTCONN; + } + bedata = dev_get_drvdata(_front_dev->dev); + + map = (struct sock_mapping *) sock->sk->sk_send_head; + if (!map) { + pvcalls_exit(); + return -ENOTSOCK; + } + + mutex_lock(>active.in_mutex); + if (len > XEN_FLEX_RING_SIZE(PVCALLS_RING_ORDER)) + len = XEN_FLEX_RING_SIZE(PVCALLS_RING_ORDER); + + while (!(flags & MSG_DONTWAIT) && !pvcalls_front_read_todo(map)) { + wait_event_interruptible(map->active.inflight_conn_req, +pvcalls_front_read_todo(map)); + } + ret = __read_ring(map->active.ring, >active.data, + >msg_iter, len, flags); + + if (ret > 0) + notify_remote_via_irq(map->active.irq); + if (ret == 0) + ret = (flags & MSG_DONTWAIT) ? -EAGAIN : 0; + if (ret == -ENOTCONN) + ret = 0; + + mutex_unlock(>active.in_mutex); + pvcalls_exit(); + return ret; +} + int pvcalls_front_bind(struct socket *sock, struct sockaddr *addr, int addr_len) { struct pvcalls_bedata *bedata; diff --git a/drivers/xen/pvcalls-front.h b/drivers/xen/pvcalls-front.h index d937c24..de24041 100644 --- a/drivers/xen/pvcalls-front.h +++ b/drivers/xen/pvcalls-front.h @@ -16,5
[Xen-devel] [PATCH v6 09/13] xen/pvcalls: implement sendmsg
Send data to an active socket by copying data to the "out" ring. Take the active socket out_mutex so that only one function can access the ring at any given time. If not enough room is available on the ring, rather than returning immediately or sleep-waiting, spin for up to 5000 cycles. This small optimization turns out to improve performance significantly. Signed-off-by: Stefano StabelliniReviewed-by: Boris Ostrovsky CC: boris.ostrov...@oracle.com CC: jgr...@suse.com --- drivers/xen/pvcalls-front.c | 118 drivers/xen/pvcalls-front.h | 3 ++ 2 files changed, 121 insertions(+) diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c index 507c6a8..27b7970 100644 --- a/drivers/xen/pvcalls-front.c +++ b/drivers/xen/pvcalls-front.c @@ -29,6 +29,7 @@ #define PVCALLS_INVALID_ID UINT_MAX #define PVCALLS_RING_ORDER XENBUS_MAX_RING_GRANT_ORDER #define PVCALLS_NR_RSP_PER_RING __CONST_RING_SIZE(xen_pvcalls, XEN_PAGE_SIZE) +#define PVCALLS_FRONT_MAX_SPIN 5000 struct pvcalls_bedata { struct xen_pvcalls_front_ring ring; @@ -99,6 +100,23 @@ static inline int get_request(struct pvcalls_bedata *bedata, int *req_id) return 0; } +static bool pvcalls_front_write_todo(struct sock_mapping *map) +{ + struct pvcalls_data_intf *intf = map->active.ring; + RING_IDX cons, prod, size = XEN_FLEX_RING_SIZE(PVCALLS_RING_ORDER); + int32_t error; + + error = intf->out_error; + if (error == -ENOTCONN) + return false; + if (error != 0) + return true; + + cons = intf->out_cons; + prod = intf->out_prod; + return !!(size - pvcalls_queued(prod, cons, size)); +} + static irqreturn_t pvcalls_front_event_handler(int irq, void *dev_id) { struct xenbus_device *dev = dev_id; @@ -363,6 +381,106 @@ int pvcalls_front_connect(struct socket *sock, struct sockaddr *addr, return ret; } +static int __write_ring(struct pvcalls_data_intf *intf, + struct pvcalls_data *data, + struct iov_iter *msg_iter, + int len) +{ + RING_IDX cons, prod, size, masked_prod, masked_cons; + RING_IDX array_size = XEN_FLEX_RING_SIZE(PVCALLS_RING_ORDER); + int32_t error; + + error = intf->out_error; + if (error < 0) + return error; + cons = intf->out_cons; + prod = intf->out_prod; + /* read indexes before continuing */ + virt_mb(); + + size = pvcalls_queued(prod, cons, array_size); + if (size >= array_size) + return -EINVAL; + if (len > array_size - size) + len = array_size - size; + + masked_prod = pvcalls_mask(prod, array_size); + masked_cons = pvcalls_mask(cons, array_size); + + if (masked_prod < masked_cons) { + copy_from_iter(data->out + masked_prod, len, msg_iter); + } else { + if (len > array_size - masked_prod) { + copy_from_iter(data->out + masked_prod, + array_size - masked_prod, msg_iter); + copy_from_iter(data->out, + len - (array_size - masked_prod), + msg_iter); + } else { + copy_from_iter(data->out + masked_prod, len, msg_iter); + } + } + /* write to ring before updating pointer */ + virt_wmb(); + intf->out_prod += len; + + return len; +} + +int pvcalls_front_sendmsg(struct socket *sock, struct msghdr *msg, + size_t len) +{ + struct pvcalls_bedata *bedata; + struct sock_mapping *map; + int sent, tot_sent = 0; + int count = 0, flags; + + flags = msg->msg_flags; + if (flags & (MSG_CONFIRM|MSG_DONTROUTE|MSG_EOR|MSG_OOB)) + return -EOPNOTSUPP; + + pvcalls_enter(); + if (!pvcalls_front_dev) { + pvcalls_exit(); + return -ENOTCONN; + } + bedata = dev_get_drvdata(_front_dev->dev); + + map = (struct sock_mapping *) sock->sk->sk_send_head; + if (!map) { + pvcalls_exit(); + return -ENOTSOCK; + } + + mutex_lock(>active.out_mutex); + if ((flags & MSG_DONTWAIT) && !pvcalls_front_write_todo(map)) { + mutex_unlock(>active.out_mutex); + pvcalls_exit(); + return -EAGAIN; + } + if (len > INT_MAX) + len = INT_MAX; + +again: + count++; + sent = __write_ring(map->active.ring, + >active.data, >msg_iter, + len); + if (sent > 0) { + len -= sent; + tot_sent += sent; + notify_remote_via_irq(map->active.irq); + } + if (sent >=
[Xen-devel] [PATCH v6 05/13] xen/pvcalls: implement connect command
Send PVCALLS_CONNECT to the backend. Allocate a new ring and evtchn for the active socket. Introduce fields in struct sock_mapping to keep track of active sockets. Introduce a waitqueue to allow the frontend to wait on data coming from the backend on the active socket (recvmsg command). Two mutexes (one of reads and one for writes) will be used to protect the active socket in and out rings from concurrent accesses. Signed-off-by: Stefano StabelliniReviewed-by: Boris Ostrovsky CC: boris.ostrov...@oracle.com CC: jgr...@suse.com --- drivers/xen/pvcalls-front.c | 162 drivers/xen/pvcalls-front.h | 2 + 2 files changed, 164 insertions(+) diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c index 8b39be5..c426d41 100644 --- a/drivers/xen/pvcalls-front.c +++ b/drivers/xen/pvcalls-front.c @@ -13,6 +13,10 @@ */ #include +#include +#include + +#include #include #include @@ -55,6 +59,18 @@ struct sock_mapping { bool active_socket; struct list_head list; struct socket *sock; + union { + struct { + int irq; + grant_ref_t ref; + struct pvcalls_data_intf *ring; + struct pvcalls_data data; + struct mutex in_mutex; + struct mutex out_mutex; + + wait_queue_head_t inflight_conn_req; + } active; + }; }; static inline int get_request(struct pvcalls_bedata *bedata, int *req_id) @@ -117,6 +133,18 @@ static void pvcalls_front_free_map(struct pvcalls_bedata *bedata, { } +static irqreturn_t pvcalls_front_conn_handler(int irq, void *sock_map) +{ + struct sock_mapping *map = sock_map; + + if (map == NULL) + return IRQ_HANDLED; + + wake_up_interruptible(>active.inflight_conn_req); + + return IRQ_HANDLED; +} + int pvcalls_front_socket(struct socket *sock) { struct pvcalls_bedata *bedata; @@ -192,6 +220,132 @@ int pvcalls_front_socket(struct socket *sock) return ret; } +static int create_active(struct sock_mapping *map, int *evtchn) +{ + void *bytes; + int ret = -ENOMEM, irq = -1, i; + + *evtchn = -1; + init_waitqueue_head(>active.inflight_conn_req); + + map->active.ring = (struct pvcalls_data_intf *) + __get_free_page(GFP_KERNEL | __GFP_ZERO); + if (map->active.ring == NULL) + goto out_error; + map->active.ring->ring_order = PVCALLS_RING_ORDER; + bytes = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, + PVCALLS_RING_ORDER); + if (bytes == NULL) + goto out_error; + for (i = 0; i < (1 << PVCALLS_RING_ORDER); i++) + map->active.ring->ref[i] = gnttab_grant_foreign_access( + pvcalls_front_dev->otherend_id, + pfn_to_gfn(virt_to_pfn(bytes) + i), 0); + + map->active.ref = gnttab_grant_foreign_access( + pvcalls_front_dev->otherend_id, + pfn_to_gfn(virt_to_pfn((void *)map->active.ring)), 0); + + map->active.data.in = bytes; + map->active.data.out = bytes + + XEN_FLEX_RING_SIZE(PVCALLS_RING_ORDER); + + ret = xenbus_alloc_evtchn(pvcalls_front_dev, evtchn); + if (ret) + goto out_error; + irq = bind_evtchn_to_irqhandler(*evtchn, pvcalls_front_conn_handler, + 0, "pvcalls-frontend", map); + if (irq < 0) { + ret = irq; + goto out_error; + } + + map->active.irq = irq; + map->active_socket = true; + mutex_init(>active.in_mutex); + mutex_init(>active.out_mutex); + + return 0; + +out_error: + if (irq >= 0) + unbind_from_irqhandler(irq, map); + else if (*evtchn >= 0) + xenbus_free_evtchn(pvcalls_front_dev, *evtchn); + kfree(map->active.data.in); + kfree(map->active.ring); + return ret; +} + +int pvcalls_front_connect(struct socket *sock, struct sockaddr *addr, + int addr_len, int flags) +{ + struct pvcalls_bedata *bedata; + struct sock_mapping *map = NULL; + struct xen_pvcalls_request *req; + int notify, req_id, ret, evtchn; + + if (addr->sa_family != AF_INET || sock->type != SOCK_STREAM) + return -EOPNOTSUPP; + + pvcalls_enter(); + if (!pvcalls_front_dev) { + pvcalls_exit(); + return -ENOTCONN; + } + + bedata = dev_get_drvdata(_front_dev->dev); + + map = (struct sock_mapping *)sock->sk->sk_send_head; + if (!map) { + pvcalls_exit(); + return -ENOTSOCK; + } + + spin_lock(>socket_lock); + ret = get_request(bedata,
[Xen-devel] [PATCH v6 04/13] xen/pvcalls: implement socket command and handle events
Send a PVCALLS_SOCKET command to the backend, use the masked req_prod_pvt as req_id. This way, req_id is guaranteed to be between 0 and PVCALLS_NR_REQ_PER_RING. We already have a slot in the rsp array ready for the response, and there cannot be two outstanding responses with the same req_id. Wait for the response by waiting on the inflight_req waitqueue and check for the req_id field in rsp[req_id]. Use atomic accesses and barriers to read the field. Note that the barriers are simple smp barriers (as opposed to virt barriers) because they are for internal frontend synchronization, not frontend<->backend communication. Once a response is received, clear the corresponding rsp slot by setting req_id to PVCALLS_INVALID_ID. Note that PVCALLS_INVALID_ID is invalid only from the frontend point of view. It is not part of the PVCalls protocol. pvcalls_front_event_handler is in charge of copying responses from the ring to the appropriate rsp slot. It is done by copying the body of the response first, then by copying req_id atomically. After the copies, wake up anybody waiting on waitqueue. socket_lock protects accesses to the ring. Create a new struct sock_mapping and convert the pointer into an uint64_t and use it as id for the new socket to pass to the backend. The struct will be fully initialized later on connect or bind. In this patch the struct sock_mapping is empty, the fields will be added by the next patch. sock->sk->sk_send_head is not used for ip sockets: reuse the field to store a pointer to the struct sock_mapping corresponding to the socket. This way, we can easily get the struct sock_mapping from the struct socket. Signed-off-by: Stefano StabelliniReviewed-by: Boris Ostrovsky CC: boris.ostrov...@oracle.com CC: jgr...@suse.com --- drivers/xen/pvcalls-front.c | 133 drivers/xen/pvcalls-front.h | 8 +++ 2 files changed, 141 insertions(+) create mode 100644 drivers/xen/pvcalls-front.h diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c index 530ef05..8b39be5 100644 --- a/drivers/xen/pvcalls-front.c +++ b/drivers/xen/pvcalls-front.c @@ -20,6 +20,8 @@ #include #include +#include "pvcalls-front.h" + #define PVCALLS_INVALID_ID UINT_MAX #define PVCALLS_RING_ORDER XENBUS_MAX_RING_GRANT_ORDER #define PVCALLS_NR_RSP_PER_RING __CONST_RING_SIZE(xen_pvcalls, XEN_PAGE_SIZE) @@ -49,8 +51,64 @@ struct pvcalls_bedata { atomic_dec(_refcount); \ } +struct sock_mapping { + bool active_socket; + struct list_head list; + struct socket *sock; +}; + +static inline int get_request(struct pvcalls_bedata *bedata, int *req_id) +{ + *req_id = bedata->ring.req_prod_pvt & (RING_SIZE(>ring) - 1); + if (RING_FULL(>ring) || + bedata->rsp[*req_id].req_id != PVCALLS_INVALID_ID) + return -EAGAIN; + return 0; +} + static irqreturn_t pvcalls_front_event_handler(int irq, void *dev_id) { + struct xenbus_device *dev = dev_id; + struct pvcalls_bedata *bedata; + struct xen_pvcalls_response *rsp; + uint8_t *src, *dst; + int req_id = 0, more = 0, done = 0; + + if (dev == NULL) + return IRQ_HANDLED; + + pvcalls_enter(); + bedata = dev_get_drvdata(>dev); + if (bedata == NULL) { + pvcalls_exit(); + return IRQ_HANDLED; + } + +again: + while (RING_HAS_UNCONSUMED_RESPONSES(>ring)) { + rsp = RING_GET_RESPONSE(>ring, bedata->ring.rsp_cons); + + req_id = rsp->req_id; + dst = (uint8_t *)>rsp[req_id] + sizeof(rsp->req_id); + src = (uint8_t *)rsp + sizeof(rsp->req_id); + memcpy(dst, src, sizeof(*rsp) - sizeof(rsp->req_id)); + /* +* First copy the rest of the data, then req_id. It is +* paired with the barrier when accessing bedata->rsp. +*/ + smp_wmb(); + bedata->rsp[req_id].req_id = rsp->req_id; + + done = 1; + bedata->ring.rsp_cons++; + } + + RING_FINAL_CHECK_FOR_RESPONSES(>ring, more); + if (more) + goto again; + if (done) + wake_up(>inflight_req); + pvcalls_exit(); return IRQ_HANDLED; } @@ -59,6 +117,81 @@ static void pvcalls_front_free_map(struct pvcalls_bedata *bedata, { } +int pvcalls_front_socket(struct socket *sock) +{ + struct pvcalls_bedata *bedata; + struct sock_mapping *map = NULL; + struct xen_pvcalls_request *req; + int notify, req_id, ret; + + /* +* PVCalls only supports domain AF_INET, +* type SOCK_STREAM and protocol 0 sockets for now. +* +* Check socket type here, AF_INET and protocol checks are done +* by the caller. +*/ + if (sock->type != SOCK_STREAM) + return
[Xen-devel] [PATCH v6 08/13] xen/pvcalls: implement accept command
Introduce a waitqueue to allow only one outstanding accept command at any given time and to implement polling on the passive socket. Introduce a flags field to keep track of in-flight accept and poll commands. Send PVCALLS_ACCEPT to the backend. Allocate a new active socket. Make sure that only one accept command is executed at any given time by setting PVCALLS_FLAG_ACCEPT_INFLIGHT and waiting on the inflight_accept_req waitqueue. Convert the new struct sock_mapping pointer into an uint64_t and use it as id for the new socket to pass to the backend. Check if the accept call is non-blocking: in that case after sending the ACCEPT command to the backend store the sock_mapping pointer of the new struct and the inflight req_id then return -EAGAIN (which will respond only when there is something to accept). Next time accept is called, we'll check if the ACCEPT command has been answered, if so we'll pick up where we left off, otherwise we return -EAGAIN again. Note that, differently from the other commands, we can use wait_event_interruptible (instead of wait_event) in the case of accept as we are able to track the req_id of the ACCEPT response that we are waiting. Signed-off-by: Stefano StabelliniReviewed-by: Boris Ostrovsky CC: boris.ostrov...@oracle.com CC: jgr...@suse.com --- drivers/xen/pvcalls-front.c | 145 drivers/xen/pvcalls-front.h | 3 + 2 files changed, 148 insertions(+) diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c index f616378..507c6a8 100644 --- a/drivers/xen/pvcalls-front.c +++ b/drivers/xen/pvcalls-front.c @@ -76,6 +76,16 @@ struct sock_mapping { #define PVCALLS_STATUS_BIND 1 #define PVCALLS_STATUS_LISTEN2 uint8_t status; + /* +* Internal state-machine flags. +* Only one accept operation can be inflight for a socket. +* Only one poll operation can be inflight for a given socket. +*/ +#define PVCALLS_FLAG_ACCEPT_INFLIGHT 0 + uint8_t flags; + uint32_t inflight_req_id; + struct sock_mapping *accept_map; + wait_queue_head_t inflight_accept_req; } passive; }; }; @@ -391,6 +401,8 @@ int pvcalls_front_bind(struct socket *sock, struct sockaddr *addr, int addr_len) memcpy(req->u.bind.addr, addr, sizeof(*addr)); req->u.bind.len = addr_len; + init_waitqueue_head(>passive.inflight_accept_req); + map->active_socket = false; bedata->ring.req_prod_pvt++; @@ -469,6 +481,139 @@ int pvcalls_front_listen(struct socket *sock, int backlog) return ret; } +int pvcalls_front_accept(struct socket *sock, struct socket *newsock, int flags) +{ + struct pvcalls_bedata *bedata; + struct sock_mapping *map; + struct sock_mapping *map2 = NULL; + struct xen_pvcalls_request *req; + int notify, req_id, ret, evtchn, nonblock; + + pvcalls_enter(); + if (!pvcalls_front_dev) { + pvcalls_exit(); + return -ENOTCONN; + } + bedata = dev_get_drvdata(_front_dev->dev); + + map = (struct sock_mapping *) sock->sk->sk_send_head; + if (!map) { + pvcalls_exit(); + return -ENOTSOCK; + } + + if (map->passive.status != PVCALLS_STATUS_LISTEN) { + pvcalls_exit(); + return -EINVAL; + } + + nonblock = flags & SOCK_NONBLOCK; + /* +* Backend only supports 1 inflight accept request, will return +* errors for the others +*/ + if (test_and_set_bit(PVCALLS_FLAG_ACCEPT_INFLIGHT, +(void *)>passive.flags)) { + req_id = READ_ONCE(map->passive.inflight_req_id); + if (req_id != PVCALLS_INVALID_ID && + READ_ONCE(bedata->rsp[req_id].req_id) == req_id) { + map2 = map->passive.accept_map; + goto received; + } + if (nonblock) { + pvcalls_exit(); + return -EAGAIN; + } + if (wait_event_interruptible(map->passive.inflight_accept_req, + !test_and_set_bit(PVCALLS_FLAG_ACCEPT_INFLIGHT, + (void *)>passive.flags))) { + pvcalls_exit(); + return -EINTR; + } + } + + spin_lock(>socket_lock); + ret = get_request(bedata, _id); + if (ret < 0) { + clear_bit(PVCALLS_FLAG_ACCEPT_INFLIGHT, + (void *)>passive.flags); + spin_unlock(>socket_lock); + pvcalls_exit(); + return ret; + } + map2 = kzalloc(sizeof(*map2), GFP_KERNEL);
[Xen-devel] [PATCH v6 00/13] introduce the Xen PV Calls frontend
Hi all, this series introduces the frontend for the newly introduced PV Calls procotol. PV Calls is a paravirtualized protocol that allows the implementation of a set of POSIX functions in a different domain. The PV Calls frontend sends POSIX function calls to the backend, which implements them and returns a value to the frontend and acts on the function call. For more information about PV Calls, please read: https://xenbits.xen.org/docs/unstable/misc/pvcalls.html This patch series only implements the frontend driver. It doesn't attempt to redirect POSIX calls to it. The functions exported in pvcalls-front.h are meant to be used for that. A separate patch series will be sent to use them and hook them into the system. Changes in v6: - rename PVCALLS_NR_REQ_PER_RING to PVCALLS_NR_RSP_PER_RING - remove spin_lock/unlock around list_del_init in pvcalls_front_remove - change list_del_init into list_del in pvcalls_front_remove, pvcalls_front_free_map and pvcalls_front_release - add reviwed-bys - return EOPNOTSUPP when functions are unimplemented in pvcalls-front - return ret on error in pvcalls_front_accept - return an error from __write_ring if size >= array_size - return 0 instead of -EAGAIN from pvcalls_front_recvmsg when nothing was read - add bool locked parameter to pvcalls_front_free_map - move "kfree(map)" to pvcalls_front_free_map - remove socketpass_mappings, now unused Stefano Stabellini (13): xen/pvcalls: introduce the pvcalls xenbus frontend xen/pvcalls: implement frontend disconnect xen/pvcalls: connect to the backend xen/pvcalls: implement socket command and handle events xen/pvcalls: implement connect command xen/pvcalls: implement bind command xen/pvcalls: implement listen command xen/pvcalls: implement accept command xen/pvcalls: implement sendmsg xen/pvcalls: implement recvmsg xen/pvcalls: implement poll command xen/pvcalls: implement release command xen: introduce a Kconfig option to enable the pvcalls frontend drivers/xen/Kconfig |9 + drivers/xen/Makefile|1 + drivers/xen/pvcalls-front.c | 1273 +++ drivers/xen/pvcalls-front.h | 28 + 4 files changed, 1311 insertions(+) create mode 100644 drivers/xen/pvcalls-front.c create mode 100644 drivers/xen/pvcalls-front.h ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC 0/4] TEE mediator framework + OP-TEE mediator
Hi, On 23/10/17 21:11, Volodymyr Babchuk wrote: On Mon, Oct 23, 2017 at 05:59:44PM +0100, Julien Grall wrote: Hi Volodymyr, Hi Julien, Let me begin the e-mail with I am not totally adversed to putting the TEE mediator in Xen. At the moment, I am trying to understand the whole picture. Thanks for clarification. This is really reassuring :) In my turn, I'm not totally against TEE mediators in stubdoms. I'm only concerned about required efforts. On 20/10/17 18:37, Volodymyr Babchuk wrote: On Fri, Oct 20, 2017 at 02:11:14PM +0100, Julien Grall wrote: On 17/10/17 16:59, Volodymyr Babchuk wrote: On Mon, Oct 16, 2017 at 01:00:21PM +0100, Julien Grall wrote: On 11/10/17 20:01, Volodymyr Babchuk wrote: I want to present TEE mediator, that was discussed earlier ([1]). I selected design with built-in mediators. This is easiest way, it removes many questions, it is easy to implement and maintain (at least I hope so). Well, it may close the technical questions but still leave the security impact unanswered. I would have appreciated a summary of each approach and explain the pros/cons. This is the most secure way also. In terms of trust between guests and Xen at least. I'm worked with OP-TEE guys mostly, so when I hear about "security", my first thoughts are "Can TEE OS trust to XEN as a mediator? Can TEE client trust to XEN as a mediator?". And with current approach answer is "yes, they can, especially if XEN is a part of a chain of trust". But you probably wanted to ask "Can guest compromise whole system by using TEE mediator or TEE OS?". This is an interesting question. First let's discuss requirements for a TEE mediator. So, mediator should be able to: * Receive request to handle trapped SMC. This request should include user registers + some information about guest (at least domain id). * Pin/unpin domain memory pages. * Map domain memory pages into own address space with RW access. * Issue real SMC to a TEE. * Receive information about guest creation and destruction. * (Probably) inject IRQs into a domain (this can be not a requester domain, but some other domain, that also called to TEE). This is a minimal list of requirements. I think, this should be enough to implement mediator for OP-TEE. But I can't say for sure for other TEEs. Let's consider possible approaches: 1. Mediator right in XEN, works at EL2. Pros: * Mediator can use all XEN APIs * As mediator resides in XEN, it can be checked together with XEN for a validity (trusted boot). * Mediator is initialized before Dom0. Dom0 can work with a TEE. * No extra context switches, no special ABI between XEN and mediator. Cons: * Because it lives in EL2, it can compromise whole hypervisor, if there is a security bug in mediator code. * No support for closed source TEEs. Another cons is you assume TEE API is fully stable and will not change. Imagine a new function is added, or a vendor decided to hence with a new set of API. How will you know Xen is safe to use it? With whitelisting, as you correctly suggested below. XEN will process only know requests. Anything that looks unfimiliar should be rejected. Let's imagine the guest is running on a platform with a newer version of TEE. This guest will probe the version of OP-TEE and knows the new function is present. This request will be handled mediator. At this moment, OP-TEE client does not use versions. Instead it uses capability flags. So, mediator should filter all unknown caps. This will force guest to use only supported subset of features. One more question. Does it mean new functions will never be added in current capabilities? If, in the future, client will relly on versions (i.e. due to dramatic protocol change), mediator can either downgrade version or refuse to work at all. Makes sense. If as you said Xen is using a whitelist, this means the hypervisor will return unimplemented. How do you expect the guest to behave in that case? As I said above, guest should downgrade to supported features subset. Note that I think a whitelist is a good idea, but I think we need to think a bit more about the implication. At least now OP-TEE is designed in a such way, that it is compatible in both ways. I'm sure that future OP-TEE development will be done with virtualization support in mind, so it will not break existing setups. It would be good to have the two communities talking together. So we can make sure the virtualization support is not going in the wrong direction. Similarly, it would be nice that someone from the OP-TEE maintainers give feedback on the approach suggested in Xen. If it is not safe, this means you have a whitelist solution and therefore tie Xen to a specific OP-TEE version. So if you need to use a new function you would need to upgrade Xen making the code of using new version potentially high. Yes, any ABI change between OP-TEE and its clients will require mediator
Re: [Xen-devel] [PATCH v5.1 7/8] os-posix: Provide new -runas : facility
On Fri, Oct 20, 2017 at 02:38:21PM +0100, Ian Jackson wrote: > +static bool os_parse_runas_uid_gid(const char *optarg) > +{ > +unsigned long lv; > +char *ep; > +uid_t got_uid; > +gid_t got_gid; > +int rc; > + > +errno = 0; > +lv = strtoul(optarg, , 0); /* can't qemu_strtoul, want *ep==':' */ Should strtoul base be 10? If that matter. > +got_uid = lv; /* overflow here is ID in C99 */ > +if (errno || *ep != ':' || got_uid != lv || got_uid == (uid_t)-1) { > +return false; > +} > + > +lv = 0; > +rc = qemu_strtoul(ep + 1, 0, 0, ); > +got_gid = lv; /* overflow here is ID in C99 */ > +if (rc || got_gid != lv || got_gid == (gid_t)-1) { > +return false; > +} > + > +user_uid = got_uid; > +user_gid = got_gid; > +return true; > +} > + > /* > * Parse OS specific command line options. > * return 0 if option handled, -1 otherwise > @@ -145,8 +175,10 @@ void os_parse_cmd_args(int index, const char *optarg) > #endif > case QEMU_OPTION_runas: > user_pwd = getpwnam(optarg); > -if (!user_pwd) { > -fprintf(stderr, "User \"%s\" doesn't exist\n", optarg); > +if (!user_pwd && !os_parse_runas_uid_gid(optarg)) { > +fprintf(stderr, > +"User \"%s\" doesn't exist (and is not .)\n", The error message have not been update, I think it should be : > +optarg); > exit(1); > } > break; With the error message fix: Reviewed-by: Anthony PERARD-- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5 12/13] xen/pvcalls: implement release command
On Tue, 24 Oct 2017, Boris Ostrovsky wrote: > (I just noticed that I missed this patch, sorry) Thanks for the review! > On 10/06/2017 08:30 PM, Stefano Stabellini wrote: > > Send PVCALLS_RELEASE to the backend and wait for a reply. Take both > > in_mutex and out_mutex to avoid concurrent accesses. Then, free the > > socket. > > > > For passive sockets, check whether we have already pre-allocated an > > active socket for the purpose of being accepted. If so, free that as > > well. > > > > Signed-off-by: Stefano Stabellini> > CC: boris.ostrov...@oracle.com > > CC: jgr...@suse.com > > --- > > drivers/xen/pvcalls-front.c | 98 > > + > > drivers/xen/pvcalls-front.h | 1 + > > 2 files changed, 99 insertions(+) > > > > diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c > > index aca2b32..9beb34d 100644 > > --- a/drivers/xen/pvcalls-front.c > > +++ b/drivers/xen/pvcalls-front.c > > @@ -200,6 +200,19 @@ static irqreturn_t pvcalls_front_event_handler(int > > irq, void *dev_id) > > static void pvcalls_front_free_map(struct pvcalls_bedata *bedata, > >struct sock_mapping *map) > > { > > + int i; > > + > > + unbind_from_irqhandler(map->active.irq, map); > > + > > + spin_lock(>socket_lock); > > + if (!list_empty(>list)) > > + list_del_init(>list); > > As with patch 2, do you need to init this? In fact, do you need to do > anything with the list? We are about to free the map (and so maybe bring > 'kfree(map)" inside here, btw?) > > And what does it mean if the list is not empty? Is it OK to free the map? Yes, list_del_init should be just list_del in this case. These two lines are only there to remove the map from socket_mappings if the map is part of one. Normally, map->list should NOT be empty. Yes, kfree(map) could be in pvcalls_front_free_map, I'll make the change. I have just noticed that we have a socketpass_mappings in struct pvcalls_bedata that used to be used in earlier versions of this series, but it is now unused. Today, we just use socket_mappings for both active and passive sockets. I'll remove it and fix pvcalls_front_remove accordingly. > > + spin_unlock(>socket_lock); > > + > > + for (i = 0; i < (1 << PVCALLS_RING_ORDER); i++) > > + gnttab_end_foreign_access(map->active.ring->ref[i], 0, 0); > > + gnttab_end_foreign_access(map->active.ref, 0, 0); > > + free_page((unsigned long)map->active.ring); > > } > > > > static irqreturn_t pvcalls_front_conn_handler(int irq, void *sock_map) > > @@ -968,6 +981,91 @@ unsigned int pvcalls_front_poll(struct file *file, > > struct socket *sock, > > return ret; > > } > > > > > > + > > + if (map->active_socket) { > > + /* > > +* Set in_error and wake up inflight_conn_req to force > > +* recvmsg waiters to exit. > > +*/ > > + map->active.ring->in_error = -EBADF; > > + wake_up_interruptible(>active.inflight_conn_req); > > + > > + /* > > +* Wait until there are no more waiters on the mutexes. > > +* We know that no new waiters can be added because sk_send_head > > +* is set to NULL -- we only need to wait for the existing > > +* waiters to return. > > +*/ > > + while (!mutex_trylock(>active.in_mutex) || > > + !mutex_trylock(>active.out_mutex)) > > + cpu_relax(); > > + > > + pvcalls_front_free_map(bedata, map); > > + kfree(map); > > + } else { > > + spin_lock(>socket_lock); > > + if (READ_ONCE(map->passive.inflight_req_id) != > > + PVCALLS_INVALID_ID) { > > + pvcalls_front_free_map(bedata, > > pvcalls_front_free_map will try to grab bedata->socket_lock, which we are > already holding. This is a mistake, well spotted! I'll add a boolean "locked" parameter to pvcalls_front_free_map. If (locked), pvcalls_front_free_map won't spin_lock. > > > + map->passive.accept_map); > > + kfree(map->passive.accept_map); > > + } > > + list_del_init(>list); > > Again, no init? Yes, I'll remove > > + kfree(map); > > + spin_unlock(>socket_lock); > > + } > > + WRITE_ONCE(bedata->rsp[req_id].req_id, PVCALLS_INVALID_ID); > > > ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 1/2] arm/xen: vpl011: Fix the slow early console SBSA UART output
From: Bhupinder ThakurThe early console output uses pl011_early_write() to write data. This function waits for BUSY bit to get cleared before writing the next byte. In the SBSA UART emulation logic, the BUSY bit was set as soon one byte was written in the FIFO and it remained set until the FIFO was emptied. This meant that the output was delayed as each character needed the BUSY to get cleared. Since the SBSA UART is getting emulated in Xen using ring buffers, it ensures that once the data is enqueued in the FIFO, it will be received by xenconsole so it is safe to set the BUSY bit only when FIFO becomes full. This will ensure that pl011_early_write() is not delayed unduly to write the data. Signed-off-by: Bhupinder Thakur Reviewed-by: Andre Przywara Signed-off-by: Andre Przywara --- xen/arch/arm/vpl011.c | 21 - 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c index f7ddccb42a..0b0743679f 100644 --- a/xen/arch/arm/vpl011.c +++ b/xen/arch/arm/vpl011.c @@ -159,9 +159,15 @@ static void vpl011_write_data(struct domain *d, uint8_t data) { vpl011->uartfr |= TXFF; vpl011->uartris &= ~TXI; -} -vpl011->uartfr |= BUSY; +/* + * This bit is set only when FIFO becomes full. This ensures that + * the SBSA UART driver can write the early console data as fast as + * possible, without waiting for the BUSY bit to get cleared before + * writing each byte. + */ +vpl011->uartfr |= BUSY; +} vpl011->uartfr &= ~TXFE; @@ -371,11 +377,16 @@ static void vpl011_data_avail(struct domain *d) { vpl011->uartfr &= ~TXFF; vpl011->uartris |= TXI; + +/* + * Clear the BUSY bit as soon as space becomes available + * so that the SBSA UART driver can start writing more data + * without any further delay. + */ +vpl011->uartfr &= ~BUSY; + if ( out_ring_qsize == 0 ) -{ -vpl011->uartfr &= ~BUSY; vpl011->uartfr |= TXFE; -} } vpl011_update_interrupt_status(d); -- 2.14.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 2/2] arm/xen: vpl011: Fix SBSA UART interrupt assertion
From: Bhupinder ThakurWith the current SBSA UART emulation, streaming larger amounts of data (as caused by "find /", for instance) can lead to character loses. This is due to the OUT ring buffer getting full, because we change the TX interrupt bit only when the FIFO is actually full, and not already when it's half-way filled, as the Linux driver expects. The SBSA spec does not explicitly state this, but we assume that an SBSA compliant UART uses the PL011 default "interrupt FIFO level select register" value of "1/2 way". The Linux driver certainly makes this assumption, so it expect to be able to write a number of characters after the TX interrupt line has been asserted. On a similar issue we have the same wrong behaviour on the receive side. However changing the RX interrupt to trigger on reaching half of the FIFO level will lead to lag, because the guest would not be notified of incoming characters until the FIFO is half way filled. This leads to inacceptible lags when typing on a terminal. Real hardware solves this issue by using the "receive timeout interrupt" (RTI), which is triggered when character reception stops for 32 baud cycles. As we cannot and do not want to emulate any timing here, we slightly abuse the timeout interrupt to notify the guest of new characters: when a new character comes in, the RTI is asserted, when the FIFO is cleared, the interrupt gets cleared. So this patch changes the emulated interrupt trigger behaviour to come as close to real hardware as possible: the RX and TX interrupt trigger when the FIFO gets half full / half empty, and the RTI interrupt signals new incoming characters. [Andre: reword commit message, introduce receive timeout interrupt, add comments] Signed-off-by: Bhupinder Thakur Reviewed-by: Andre Przywara Signed-off-by: Andre Przywara --- xen/arch/arm/vpl011.c| 131 ++- xen/include/asm-arm/vpl011.h | 2 + 2 files changed, 94 insertions(+), 39 deletions(-) diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c index 0b0743679f..6d02406acf 100644 --- a/xen/arch/arm/vpl011.c +++ b/xen/arch/arm/vpl011.c @@ -18,6 +18,9 @@ #define XEN_WANT_FLEX_CONSOLE_RING 1 +/* We assume the PL011 default of "1/2 way" for the FIFO trigger level. */ +#define SBSA_UART_FIFO_LEVEL (SBSA_UART_FIFO_SIZE / 2) + #include #include #include @@ -93,24 +96,37 @@ static uint8_t vpl011_read_data(struct domain *d) */ if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) > 0 ) { +unsigned int fifo_level; + data = intf->in[xencons_mask(in_cons, sizeof(intf->in))]; in_cons += 1; smp_mb(); intf->in_cons = in_cons; + +fifo_level = xencons_queued(in_prod, in_cons, sizeof(intf->in)); + +/* If the FIFO is now empty, we clear the receive timeout interrupt. */ +if ( fifo_level == 0 ) +{ +vpl011->uartfr |= RXFE; +vpl011->uartris &= ~RTI; +} + +/* If the FIFO is more than half empty, we clear the RX interrupt. */ +if ( fifo_level < sizeof(intf->in) - SBSA_UART_FIFO_LEVEL ) +vpl011->uartris &= ~RXI; + +vpl011_update_interrupt_status(d); } else gprintk(XENLOG_ERR, "vpl011: Unexpected IN ring buffer empty\n"); -if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) == 0 ) -{ -vpl011->uartfr |= RXFE; -vpl011->uartris &= ~RXI; -} - +/* + * We have consumed a character or the FIFO was empty, so clear the + * "FIFO full" bit. + */ vpl011->uartfr &= ~RXFF; -vpl011_update_interrupt_status(d); - VPL011_UNLOCK(d, flags); /* @@ -122,6 +138,24 @@ static uint8_t vpl011_read_data(struct domain *d) return data; } +static void vpl011_update_tx_fifo_status(struct vpl011 *vpl011, + unsigned int fifo_level) +{ +struct xencons_interface *intf = vpl011->ring_buf; +unsigned int fifo_threshold = sizeof(intf->out) - SBSA_UART_FIFO_LEVEL; + +BUILD_BUG_ON(sizeof (intf->out) < SBSA_UART_FIFO_SIZE); + +/* + * Set the TXI bit only when there is space for fifo_size/2 bytes which + * is the trigger level for asserting/de-assterting the TX interrupt. + */ +if ( fifo_level <= fifo_threshold ) +vpl011->uartris |= TXI; +else +vpl011->uartris &= ~TXI; +} + static void vpl011_write_data(struct domain *d, uint8_t data) { unsigned long flags; @@ -146,33 +180,37 @@ static void vpl011_write_data(struct domain *d, uint8_t data) if ( xencons_queued(out_prod, out_cons, sizeof(intf->out)) != sizeof (intf->out) ) { +unsigned int fifo_level; + intf->out[xencons_mask(out_prod, sizeof(intf->out))] = data; out_prod += 1; smp_wmb(); intf->out_prod =
[Xen-devel] [xen-unstable-smoke test] 115187: tolerable all pass - PUSHED
flight 115187 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/115187/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-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 version targeted for testing: xen b8acf328ac86fbb45831917a61e94be2de34294d baseline version: xen 1f2c7894dfe3d52f33655de202bd474999a1637b Last test of basis 115156 2017-10-23 17:01:28 Z0 days Testing same since 115187 2017-10-24 15:02:14 Z0 days1 attempts People who touched revisions under test: Chao GaoJan Beulich jobs: build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-amd64-amd64-xl-qemuu-debianhvm-i386 pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : + branch=xen-unstable-smoke + revision=b8acf328ac86fbb45831917a61e94be2de34294d + . ./cri-lock-repos ++ . ./cri-common +++ . ./cri-getconfig export PERLLIB=.:. PERLLIB=.:. +++ umask 002 +++ getrepos getconfig Repos perl -e ' use Osstest; readglobalconfig(); print $c{"Repos"} or die $!; ' +++ local repos=/home/osstest/repos +++ '[' -z /home/osstest/repos ']' +++ '[' '!' -d /home/osstest/repos ']' +++ echo /home/osstest/repos ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x '!=' x/home/osstest/repos/lock ']' ++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock ++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push xen-unstable-smoke b8acf328ac86fbb45831917a61e94be2de34294d + branch=xen-unstable-smoke + revision=b8acf328ac86fbb45831917a61e94be2de34294d + . ./cri-lock-repos ++ . ./cri-common +++ . ./cri-getconfig export PERLLIB=.:.:. PERLLIB=.:.:. +++ umask 002 +++ getrepos getconfig Repos perl -e ' use Osstest; readglobalconfig(); print $c{"Repos"} or die $!; ' +++ local repos=/home/osstest/repos +++ '[' -z /home/osstest/repos ']' +++ '[' '!' -d /home/osstest/repos ']' +++ echo /home/osstest/repos ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']' + . ./cri-common ++ . ./cri-getconfig +++ export PERLLIB=.:.:.:. +++ PERLLIB=.:.:.:. ++ umask 002 + select_xenbranch + case "$branch" in + tree=xen + xenbranch=xen-unstable-smoke + qemuubranch=qemu-upstream-unstable + '[' xxen = xlinux ']' + linuxbranch= + '[' xqemu-upstream-unstable = x ']' + select_prevxenbranch ++ ./cri-getprevxenbranch xen-unstable-smoke + prevxenbranch=xen-4.9-testing + '[' xb8acf328ac86fbb45831917a61e94be2de34294d = x ']' + : tested/2.6.39.x + . ./ap-common ++ : osst...@xenbits.xen.org +++ getconfig OsstestUpstream +++ perl -e ' use Osstest; readglobalconfig(); print $c{"OsstestUpstream"} or die $!; ' ++ : ++ : git://xenbits.xen.org/xen.git ++ : osst...@xenbits.xen.org:/home/xen/git/xen.git ++ : git://xenbits.xen.org/qemu-xen-traditional.git ++ : git://git.kernel.org ++ : git://git.kernel.org/pub/scm/linux/kernel/git ++ : git ++ : git://xenbits.xen.org/xtf.git ++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git ++ : git://xenbits.xen.org/xtf.git ++ : git://xenbits.xen.org/libvirt.git ++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git ++ : git://xenbits.xen.org/libvirt.git ++ : git://xenbits.xen.org/osstest/rumprun.git ++ : git ++ : git://xenbits.xen.org/osstest/rumprun.git ++ : osst...@xenbits.xen.org:/home/xen/git/osstest/rumprun.git ++ : git://git.seabios.org/seabios.git ++ : osst...@xenbits.xen.org:/home/xen/git/osstest/seabios.git ++ : git://xenbits.xen.org/osstest/seabios.git ++ : https://github.com/tianocore/edk2.git ++ : osst...@xenbits.xen.org:/home/xen/git/osstest/ovmf.git ++ :
Re: [Xen-devel] [PATCH v5 08/13] xen/pvcalls: implement accept command
On Tue, 24 Oct 2017, Boris Ostrovsky wrote: > On 10/23/2017 07:03 PM, Stefano Stabellini wrote: > > On Tue, 17 Oct 2017, Boris Ostrovsky wrote: > >> On 10/06/2017 08:30 PM, Stefano Stabellini wrote: > >>> + /* > >>> + * Backend only supports 1 inflight accept request, will return > >>> + * errors for the others > >>> + */ > >>> + if (test_and_set_bit(PVCALLS_FLAG_ACCEPT_INFLIGHT, > >>> + (void *)>passive.flags)) { > >>> + req_id = READ_ONCE(map->passive.inflight_req_id); > >>> + if (req_id != PVCALLS_INVALID_ID && > >>> + READ_ONCE(bedata->rsp[req_id].req_id) == req_id) { > >> > >> READ_ONCE (especially the second one)? I know I may sound fixated on > >> this but I really don't understand how compiler may do anything wrong if > >> straight reads were used. > >> > >> For the first case, I guess, theoretically the compiler may decide to > >> re-fetch map->passive.inflight_req_id. But even if it did, would that be > >> a problem? Both of these READ_ONCE targets are updated below before > >> PVCALLS_FLAG_ACCEPT_INFLIGHT is cleared so there should not be any > >> change between re-fetching, I think. (The only exception is the noblock > >> case, which does WRITE_ONCE that don't understand either) > > READ_ONCE is reasonably cheap: do we really want to have this kind of > > conversation every time we touch this code in the future? Personally, I > > would have used READ/WRITE_ONCE everywhere for inflight_req_id and > > req_id, because it makes the code easier to understand. > > I guess it's a matter of opinion. I actually think it's harder to read. > > But it doesn't make the code wrong so... > > > > > We have already limited their usage, but at least we have followed a set > > of guidelines. Doing further optimizations on this code seems > > unnecessary and prone to confuse the reader. > > > > > > >>> + ret = create_active(map2, ); > >>> + if (ret < 0) { > >>> + kfree(map2); > >>> + clear_bit(PVCALLS_FLAG_ACCEPT_INFLIGHT, > >>> + (void *)>passive.flags); > >>> + spin_unlock(>socket_lock); > >>> + pvcalls_exit(); > >>> + return -ENOMEM; > >> Why not ret? > > yes, good idea. > > With that fixed (and extra space removed in 'ret = create_active(map2, > );') > > Reviewed-by: Boris OstrovskyThank you! ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC 0/4] TEE mediator framework + OP-TEE mediator
Hello Stefano On Mon, Oct 23, 2017 at 02:26:56PM -0700, Stefano Stabellini wrote: > > > >This is a lot of a work. It requires changes in generic parts of XEN. > > > >I fear it will be very hard to upstream such changes, because no one > > > >sees an immediate value in them. How do you think, what are my chances > > > >to upstream this? > > > > > > It is fairly annoying to see you justifying back most of this thread with > > > "no one sees an immediate value in them". > > > > > > I am not the only maintainers in Xen, so effectively can't promise whether > > > it is going to be upstreamed. But I believe the community has been very > > > supportive so far, a lot of discussions happened (see [2]) because of the > > > OP-TEE support. So what more do you expect from us? > > I'm sorry, I didn't mean to offend you or someone else. You, guys, can > > be harsh sometimes, but I really appreciate help provided by the > > community. And I, certainly, don't ask you about any guarantees or > > something of that sort. > > > > I'm just bothered by amount of required work and by upstreaming > > process. But this is not a strong argument against mediators in > > stubdoms, I think :) > > > > Currently I'm developing virtualization support in OP-TEE, so in > > meantime we'll have much time to discuss mediators and stubdomain > > approach (if you have time). To test this feature in OP-TEE I'm > > extending this RFC, making optee.c to look like full-scale mediator. > > I need to do this anyways, to test OP-TEE. When I'll finish, I can > > show you how mediator can look like. Maybe this will persuade you to > > one or another approach. > > Hi Volodymyr, > > We really appreciate your work and we care about your use-case. We > really want this feature to be successful for you (and everybody else). > > Sorry if it doesn't always come out this way, but email conversations > can sound "harsh" sometimes. However, keep in mind that both Julien and > I are completely on your side on this work item. Please keep up with the > good work :-) Thanks :-) > > > > >Approach in this RFC is much simpler. Few hooks in arch code + additional > > > >subsystem, which can be easily turned off. > > > > > > Stefano do you have any opinion on this discussion? > > We need to start somewhere, and I think this series could be a decent > starting point. > > I think it is OK to have a small SMC filter in Xen. What Volodymyr is > suggesting looks reasonable for now. As the code grows, we might found > ourselves in the situation where we'll have to introduce stubdoms for > TEE virtualization/emulation, and I think that's OK. Possibly, we'll > have a "fast path" in Xen, only for filtering and small manipulations, > and a "slow path" in the stubdom when more complex actions are > necessary. This sounds a bit tricky, actually. If I got you right, you are proposing to split mediator into two parts. Only benefit I can see there - fast calls to OP-TEE from Dom0. That probably can work, but I need to consider all consequences... > For this series, I think we need a way to specify which domains can talk > to TEE, so that we can only allow it for a specific subset of DomUs. I > would probably use XSM for that. I am afraid, this is not possible. As other domains aren't 1:1 mapped, I need to have special translation code in mediator. Actually, I'm writing it rigth now to test my changes in OP-TEE. But event this is not enought for decent OP-TEE support. What can be done right now: 100% Dom0-only support with vanilla OP-TEE (i.e. no virtualization support in OP-TEE is needed). This is even simplier task, so I can throw out some code from this patch series. On other hand, in the future this will lead to sutiation when two mediators for the same TEE shall be supported: one, simple, in XEN, another, fully-functional in stubdom. > For the long term, I think both Volodymyr and us as maintainers need to > be prepared to introduce stubdoms for TEE emulation. It will most > probably happen as the feature-set grows. However, this small TEE > framework in Xen could still be useful, and could be the basis for > forwarding TEE requests to a stubdom for evaluation: maybe not all calls > need to be forwarded to the stubdom, some of them could go directly to > the firmware and this is where this series comes in. > > What do you think? Hmm... I can't imagine how this can work for OP-TEE. In OP-TEE protocol, there is a number of "fast" (in SMCCC terms) service calls, which called mostly during initialization (to probe UID and version, to get shared region location, to exchange caps and so on) and one "yielding" (again, SMCCC term) call for actuall TEE tasks. The later one passes arguments in command buffer (not in registers), it can cause so-called RPC returns (when OP-TEE asks Normal World to perform certain work). Most of the mediator code will be devoted to handle this one type of call. So, I don't see benefit in splitting mediator between XEN and stubdom. At least for
Re: [Xen-devel] [PATCH v3 01/46] Replace all occurances of __FUNCTION__ with __func__
On Thu, Oct 19, 2017 at 09:15:41AM -0700, Alistair Francis wrote: > Replace all occurs of __FUNCTION__ except for the check in checkpatch > with the non GCC specific __func__. > > One line in hcd-musb.c was manually tweaked to pass checkpatch. > > Signed-off-by: Alistair Francis> Cc: Gerd Hoffmann > Cc: Andrzej Zaborowski > Cc: Stefano Stabellini > Cc: Anthony Perard > Cc: John Snow > Cc: Aurelien Jarno > Cc: Yongbok Kim > Cc: Peter Crosthwaite > Cc: Stefan Hajnoczi > Cc: Fam Zheng > Cc: Juan Quintela > Cc: "Dr. David Alan Gilbert" > Cc: qemu-...@nongnu.org > Cc: qemu-bl...@nongnu.org > Cc: xen-de...@lists.xenproject.org > Reviewed-by: Eric Blake > Reviewed-by: Stefan Hajnoczi Reviewed-by: Anthony PERARD -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.10 v2 0/5] tools/dombuilder: Fixes and improvements to grant handling
Hi, I think this is 4.10 material (particularly patch #5). Juergen, would it be possible get the some feedback on this series? Cheers, On 12/10/17 20:19, Andrew Cooper wrote: A git tree version is available: http://xenbits.xen.org/gitweb/?p=people/andrewcoop/xen.git;a=shortlog;h=refs/heads/dombuilder-gnt-v2 Changes in v2: Mainly a rebase over c/s 5b42c82f "tools/libxc: Fix domid parameter types", and fixup from review comments. See individual patches for details Andrew Cooper (5): tools/dombuilder: Drop more PVH v1 leftovers tools/dombuilder: Remove clear_page() from xc_dom_boot.c tools/dombuilder: Switch to using gfn terminology for console and xenstore rings tools/dombuilder: Fix asymmetry when setting up console and xenstore rings tools/dombuilder: Prevent failures of xc_dom_gnttab_init() tools/libxc/include/xc_dom.h | 26 ++-- tools/libxc/xc_dom_arm.c | 17 ++--- tools/libxc/xc_dom_boot.c | 126 +++--- tools/libxc/xc_dom_compat_linux.c | 6 +- tools/libxc/xc_dom_core.c | 8 +++ tools/libxc/xc_dom_x86.c | 57 + tools/libxl/libxl_dom.c | 51 ++- tools/libxl/libxl_internal.h | 1 - 8 files changed, 169 insertions(+), 123 deletions(-) -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/3] x86: FS/GS base handling adjustments
On 20/10/17 15:11, Jan Beulich wrote: 1: fix GS-base-dirty determination 2: also show FS/GS base addresses when dumping registers 3: avoid FS/GS base reads Patch 1 is a bug fix which should be strongly considered for 4.10. Patch 2 has proven helpful in analyzing the original problem, so would be nice to have upstream rather sooner than later. Patch 3 is a minor performance tweak, which can easily wait until after 4.10. Signed-off-by: Jan BeulichFor the 3 patches: Release-acked-by: Julien Grall Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC v2] Add SUPPORT.md
On Fri, Sep 15, 2017 at 3:51 PM, Konrad Rzeszutek Wilkwrote: >> +### Soft-reset for PV guests > > s/PV/HVM/ Is it? I thought this was for RHEL 5 PV guests to be able to do crash kernels. >> +### Transcendent Memory >> + >> +Status: Experimental >> + >> +[XXX Add description] > > Guests with tmem drivers autoballoon memory out allowing a fluid > and dynamic memory allocation - in effect memory overcommit without > the need to swap. Only works with Linux guests (as it requires > OS drivers). But autoballooning doesn't require any support in Xen, right? I thought the TMEM support in Xen was more about the trancendent memory backends. > ..snip.. >> +### Live Patching >> + >> +Status, x86: Supported >> +Status, ARM: Experimental >> + >> +Compile time disabled > > for ARM. > > As the patch will do: > > config LIVEPATCH > - bool "Live patching support (TECH PREVIEW)" > - default n > + bool "Live patching support" > + default X86 > depends on HAS_BUILD_ID = "y" > ---help--- > Allows a running Xen hypervisor to be dynamically patched using Ack -George ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen: fix booting ballooned down hvm guest
On 24/10/17 16:59, HW42 wrote: > Juergen Gross: >> On 24/10/17 16:56, Boris Ostrovsky wrote: >>> On 10/24/2017 10:41 AM, Juergen Gross wrote: On 24/10/17 16:33, Boris Ostrovsky wrote: > On 10/24/2017 04:10 AM, Juergen Gross wrote: >> Commit 96edd61dcf44362d3ef0bed1a5361e0ac7886a63 ("xen/balloon: don't >> online new memory initially") introduced a regression when booting a >> HVM domain with memory less than mem-max: instead of ballooning down >> immediately the system would try to use the memory up to mem-max >> resulting in Xen crashing the domain. >> >> For HVM domains the current size will be reflected in Xenstore node >> memory/static-max instead of memory/target. >> >> Additionally we have to trigger the ballooning process at once. >> >> Cc:# 4.13 >> Fixes: 96edd61dcf44362d3ef0bed1a5361e0ac7886a63 ("xen/balloon: don't >>online new memory initially") >> >> Suggested-by: Boris Ostrovsky >> Signed-off-by: Juergen Gross > Reported-by: HW42 Hmm, is an anonymous Reported-by: tag okay? >>> >>> Oh, I don't know what the rules are for this kind of address. I'd >>> probably still add it but it's up to you. >> >> The docs say each tag should be: >> >> tag: Full Name optional-other-stuff >> >> I don't think HW42 can be regarded to be a full name. > > I think you really should allow pseudonymous contributions. But in my > case my nickname is anyway linked to my legal name so fell free to use: > Simon Gaiser > Thanks, will add it in V2. Juergen ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen: fix booting ballooned down hvm guest
Juergen Gross: > On 24/10/17 16:56, Boris Ostrovsky wrote: >> On 10/24/2017 10:41 AM, Juergen Gross wrote: >>> On 24/10/17 16:33, Boris Ostrovsky wrote: On 10/24/2017 04:10 AM, Juergen Gross wrote: > Commit 96edd61dcf44362d3ef0bed1a5361e0ac7886a63 ("xen/balloon: don't > online new memory initially") introduced a regression when booting a > HVM domain with memory less than mem-max: instead of ballooning down > immediately the system would try to use the memory up to mem-max > resulting in Xen crashing the domain. > > For HVM domains the current size will be reflected in Xenstore node > memory/static-max instead of memory/target. > > Additionally we have to trigger the ballooning process at once. > > Cc:# 4.13 > Fixes: 96edd61dcf44362d3ef0bed1a5361e0ac7886a63 ("xen/balloon: don't >online new memory initially") > > Suggested-by: Boris Ostrovsky > Signed-off-by: Juergen Gross Reported-by: HW42 >>> Hmm, is an anonymous Reported-by: tag okay? >> >> Oh, I don't know what the rules are for this kind of address. I'd >> probably still add it but it's up to you. > > The docs say each tag should be: > > tag: Full Name optional-other-stuff > > I don't think HW42 can be regarded to be a full name. I think you really should allow pseudonymous contributions. But in my case my nickname is anyway linked to my legal name so fell free to use: Simon Gaiser signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen: fix booting ballooned down hvm guest
On 24/10/17 16:56, Boris Ostrovsky wrote: > On 10/24/2017 10:41 AM, Juergen Gross wrote: >> On 24/10/17 16:33, Boris Ostrovsky wrote: >>> On 10/24/2017 04:10 AM, Juergen Gross wrote: Commit 96edd61dcf44362d3ef0bed1a5361e0ac7886a63 ("xen/balloon: don't online new memory initially") introduced a regression when booting a HVM domain with memory less than mem-max: instead of ballooning down immediately the system would try to use the memory up to mem-max resulting in Xen crashing the domain. For HVM domains the current size will be reflected in Xenstore node memory/static-max instead of memory/target. Additionally we have to trigger the ballooning process at once. Cc:# 4.13 Fixes: 96edd61dcf44362d3ef0bed1a5361e0ac7886a63 ("xen/balloon: don't online new memory initially") Suggested-by: Boris Ostrovsky Signed-off-by: Juergen Gross >>> Reported-by: HW42 >> Hmm, is an anonymous Reported-by: tag okay? > > Oh, I don't know what the rules are for this kind of address. I'd > probably still add it but it's up to you. The docs say each tag should be: tag: Full Name optional-other-stuff I don't think HW42 can be regarded to be a full name. Juergen ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen: fix booting ballooned down hvm guest
On 10/24/2017 10:41 AM, Juergen Gross wrote: > On 24/10/17 16:33, Boris Ostrovsky wrote: >> On 10/24/2017 04:10 AM, Juergen Gross wrote: >>> Commit 96edd61dcf44362d3ef0bed1a5361e0ac7886a63 ("xen/balloon: don't >>> online new memory initially") introduced a regression when booting a >>> HVM domain with memory less than mem-max: instead of ballooning down >>> immediately the system would try to use the memory up to mem-max >>> resulting in Xen crashing the domain. >>> >>> For HVM domains the current size will be reflected in Xenstore node >>> memory/static-max instead of memory/target. >>> >>> Additionally we have to trigger the ballooning process at once. >>> >>> Cc:# 4.13 >>> Fixes: 96edd61dcf44362d3ef0bed1a5361e0ac7886a63 ("xen/balloon: don't >>>online new memory initially") >>> >>> Suggested-by: Boris Ostrovsky >>> Signed-off-by: Juergen Gross >> Reported-by: HW42 > Hmm, is an anonymous Reported-by: tag okay? Oh, I don't know what the rules are for this kind of address. I'd probably still add it but it's up to you. -boris ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen: fix booting ballooned down hvm guest
On 24/10/17 16:33, Boris Ostrovsky wrote: > On 10/24/2017 04:10 AM, Juergen Gross wrote: >> Commit 96edd61dcf44362d3ef0bed1a5361e0ac7886a63 ("xen/balloon: don't >> online new memory initially") introduced a regression when booting a >> HVM domain with memory less than mem-max: instead of ballooning down >> immediately the system would try to use the memory up to mem-max >> resulting in Xen crashing the domain. >> >> For HVM domains the current size will be reflected in Xenstore node >> memory/static-max instead of memory/target. >> >> Additionally we have to trigger the ballooning process at once. >> >> Cc:# 4.13 >> Fixes: 96edd61dcf44362d3ef0bed1a5361e0ac7886a63 ("xen/balloon: don't >>online new memory initially") >> >> Suggested-by: Boris Ostrovsky >> Signed-off-by: Juergen Gross > > Reported-by: HW42 Hmm, is an anonymous Reported-by: tag okay? > >> --- >> drivers/xen/xen-balloon.c | 18 -- >> 1 file changed, 12 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/xen/xen-balloon.c b/drivers/xen/xen-balloon.c >> index e89136ab851e..3745748d9644 100644 >> --- a/drivers/xen/xen-balloon.c >> +++ b/drivers/xen/xen-balloon.c >> @@ -57,7 +57,7 @@ static int register_balloon(struct device *dev); >> static void watch_target(struct xenbus_watch *watch, >> const char *path, const char *token) >> { >> -unsigned long long new_target; >> +unsigned long long new_target, static_max; >> int err; >> static bool watch_fired; >> static long target_diff; >> @@ -72,13 +72,19 @@ static void watch_target(struct xenbus_watch *watch, >> * pages. PAGE_SHIFT converts bytes to pages, hence PAGE_SHIFT - 10. >> */ >> new_target >>= PAGE_SHIFT - 10; >> -if (watch_fired) { >> -balloon_set_new_target(new_target - target_diff); >> -return; >> + >> +if (!watch_fired) { >> +watch_fired = true; >> +err = xenbus_scanf(XBT_NIL, "memory", "static-max", "%llu", >> + _max); >> +if (err != 1) >> +static_max = new_target; >> +static_max >>= PAGE_SHIFT - 10; > > if you set static_max to new_target you've already done the shift, > haven't you? Aah, right. I moved reading static-max down into the if without adjustment after having it right at the start of the function initially. Thanks for catching this. >> +target_diff = xen_pv_domain() ? 0 > > Why do we special-case PV? Because the initial value of balloon_stats.target_pages is special-cased for PV, too. Juergen ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen: fix booting ballooned down hvm guest
On 10/24/2017 04:10 AM, Juergen Gross wrote: > Commit 96edd61dcf44362d3ef0bed1a5361e0ac7886a63 ("xen/balloon: don't > online new memory initially") introduced a regression when booting a > HVM domain with memory less than mem-max: instead of ballooning down > immediately the system would try to use the memory up to mem-max > resulting in Xen crashing the domain. > > For HVM domains the current size will be reflected in Xenstore node > memory/static-max instead of memory/target. > > Additionally we have to trigger the ballooning process at once. > > Cc:# 4.13 > Fixes: 96edd61dcf44362d3ef0bed1a5361e0ac7886a63 ("xen/balloon: don't >online new memory initially") > > Suggested-by: Boris Ostrovsky > Signed-off-by: Juergen Gross Reported-by: HW42 > --- > drivers/xen/xen-balloon.c | 18 -- > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/drivers/xen/xen-balloon.c b/drivers/xen/xen-balloon.c > index e89136ab851e..3745748d9644 100644 > --- a/drivers/xen/xen-balloon.c > +++ b/drivers/xen/xen-balloon.c > @@ -57,7 +57,7 @@ static int register_balloon(struct device *dev); > static void watch_target(struct xenbus_watch *watch, >const char *path, const char *token) > { > - unsigned long long new_target; > + unsigned long long new_target, static_max; > int err; > static bool watch_fired; > static long target_diff; > @@ -72,13 +72,19 @@ static void watch_target(struct xenbus_watch *watch, >* pages. PAGE_SHIFT converts bytes to pages, hence PAGE_SHIFT - 10. >*/ > new_target >>= PAGE_SHIFT - 10; > - if (watch_fired) { > - balloon_set_new_target(new_target - target_diff); > - return; > + > + if (!watch_fired) { > + watch_fired = true; > + err = xenbus_scanf(XBT_NIL, "memory", "static-max", "%llu", > +_max); > + if (err != 1) > + static_max = new_target; > + static_max >>= PAGE_SHIFT - 10; if you set static_max to new_target you've already done the shift, haven't you? > + target_diff = xen_pv_domain() ? 0 Why do we special-case PV? -boris > + : static_max - balloon_stats.target_pages; > } > > - watch_fired = true; > - target_diff = new_target - balloon_stats.target_pages; > + balloon_set_new_target(new_target - target_diff); > } > static struct xenbus_watch target_watch = { > .node = "memory/target", ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC v2 5/7] acpi:arm64: Add support for parsing IORT table
Hi Sameer, On 19/10/17 16:21, Goel, Sameer wrote: On 10/12/2017 8:06 AM, Julien Grall wrote: + +/* Redefine WARN macros */ +#undef WARN +#undef WARN_ON +#define WARN(condition, format...) ({ \ + int __ret_warn_on = !!(condition); \ + if (unlikely(__ret_warn_on)) \ + printk(format); \ + unlikely(__ret_warn_on); \ +}) Again, you should at least try to modify the common code version before deciding to redefine it here. The xen macro seems such that it explicitly tries to block a return by wrapping this macro in a loop. I had changed the common function in the last iteration and there seemed to be a pushback. I don't think there was any pushback on changing the common code. Jan Beulich validly requested to move that change in a separate patch. +#define WARN_TAINT(cond, taint, format...) WARN(cond, format) +#define WARN_ON(cond) (!!cond) #define IORT_TYPE_MASK(type) (1 << (type)) #define IORT_MSI_TYPE (1 << ACPI_IORT_NODE_ITS_GROUP) @@ -256,6 +286,13 @@ static acpi_status iort_match_node_callback(struct acpi_iort_node *node, acpi_status status; if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT) { + status = AE_NOT_IMPLEMENTED; +/* + * We need the namespace object name from dsdt to match the iort node, this Please add a "Xen: TODO:" in front. Ok. + * will need additions to the kernel xen bus notifiers. + * So, disabling the named node code till a proposal is approved. + */ +#if 0 struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL }; struct acpi_device *adev = to_acpi_device_node(dev->fwnode); struct acpi_iort_named_component *ncomp; @@ -275,11 +312,12 @@ static acpi_status iort_match_node_callback(struct acpi_iort_node *node, status = !strcmp(ncomp->device_name, buf.pointer) ? AE_OK : AE_NOT_FOUND; acpi_os_free(buf.pointer); +#endif } else if (node->type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX) { struct acpi_iort_root_complex *pci_rc; - struct pci_bus *bus; + struct pci_dev *pci_dev; Do you really need to modify the code? Wouldn't it be possible to do #define pci_bus pci_dev The pci_dev is the container for the generic device. I wanted to call this out explicitly here. We can do the above if you insist :). I agree this can be confusing to read. But that change is not justified for the goal you want to achieve. E.g importing the code from Linux and keep in sync in the future. With a comment on top of the definitions, you could explain when pci_bus = pci_dev at the moment. [...] diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c index 362d578..ad956d5 100644 --- a/xen/drivers/passthrough/arm/smmu.c +++ b/xen/drivers/passthrough/arm/smmu.c @@ -181,6 +181,7 @@ static void __iomem *devm_ioremap_resource(struct device *dev, * Xen: PCI functions * TODO: It should be implemented when PCI will be supported */ +#undef to_pci_dev Why this change? I had redefine the to_pci_dev to get the actual pci_dev struct. smmu driver does not use pci yet. That should go in a separate commit in that case with probably all stub for PCI you added (see the pci.h). The reason behind is those changes are not directly related to this patch. Patch should be kept simple and do one thing only. This makes easier to review. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5 12/13] xen/pvcalls: implement release command
(I just noticed that I missed this patch, sorry) On 10/06/2017 08:30 PM, Stefano Stabellini wrote: > Send PVCALLS_RELEASE to the backend and wait for a reply. Take both > in_mutex and out_mutex to avoid concurrent accesses. Then, free the > socket. > > For passive sockets, check whether we have already pre-allocated an > active socket for the purpose of being accepted. If so, free that as > well. > > Signed-off-by: Stefano Stabellini> CC: boris.ostrov...@oracle.com > CC: jgr...@suse.com > --- > drivers/xen/pvcalls-front.c | 98 > + > drivers/xen/pvcalls-front.h | 1 + > 2 files changed, 99 insertions(+) > > diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c > index aca2b32..9beb34d 100644 > --- a/drivers/xen/pvcalls-front.c > +++ b/drivers/xen/pvcalls-front.c > @@ -200,6 +200,19 @@ static irqreturn_t pvcalls_front_event_handler(int irq, > void *dev_id) > static void pvcalls_front_free_map(struct pvcalls_bedata *bedata, > struct sock_mapping *map) > { > + int i; > + > + unbind_from_irqhandler(map->active.irq, map); > + > + spin_lock(>socket_lock); > + if (!list_empty(>list)) > + list_del_init(>list); As with patch 2, do you need to init this? In fact, do you need to do anything with the list? We are about to free the map (and so maybe bring 'kfree(map)" inside here, btw?) And what does it mean if the list is not empty? Is it OK to free the map? > + spin_unlock(>socket_lock); > + > + for (i = 0; i < (1 << PVCALLS_RING_ORDER); i++) > + gnttab_end_foreign_access(map->active.ring->ref[i], 0, 0); > + gnttab_end_foreign_access(map->active.ref, 0, 0); > + free_page((unsigned long)map->active.ring); > } > > static irqreturn_t pvcalls_front_conn_handler(int irq, void *sock_map) > @@ -968,6 +981,91 @@ unsigned int pvcalls_front_poll(struct file *file, > struct socket *sock, > return ret; > } > > + > + if (map->active_socket) { > + /* > + * Set in_error and wake up inflight_conn_req to force > + * recvmsg waiters to exit. > + */ > + map->active.ring->in_error = -EBADF; > + wake_up_interruptible(>active.inflight_conn_req); > + > + /* > + * Wait until there are no more waiters on the mutexes. > + * We know that no new waiters can be added because sk_send_head > + * is set to NULL -- we only need to wait for the existing > + * waiters to return. > + */ > + while (!mutex_trylock(>active.in_mutex) || > +!mutex_trylock(>active.out_mutex)) > + cpu_relax(); > + > + pvcalls_front_free_map(bedata, map); > + kfree(map); > + } else { > + spin_lock(>socket_lock); > + if (READ_ONCE(map->passive.inflight_req_id) != > + PVCALLS_INVALID_ID) { > + pvcalls_front_free_map(bedata, pvcalls_front_free_map will try to grab bedata->socket_lock, which we are already holding. > +map->passive.accept_map); > + kfree(map->passive.accept_map); > + } > + list_del_init(>list); Again, no init? -boris > + kfree(map); > + spin_unlock(>socket_lock); > + } > + WRITE_ONCE(bedata->rsp[req_id].req_id, PVCALLS_INVALID_ID); > ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC v2 2/7] arm64: Add definitions for fwnode_handle
Hi Sameer, On 19/10/17 15:53, Goel, Sameer wrote: On 10/12/2017 6:45 AM, Julien Grall wrote: On 21/09/17 01:37, Sameer Goel wrote: This will be used as a device property to match the DMA capable devices with the associated SMMU. The header file is a port from linux. The code was changed to remove the types that were not needed for Xen. I think you probably want a bit more context in the commit message about implement fwnode.h in common code. Within this series, fwnode seems to only be used by Arm. So what would be the advantage to get that in xen/? Is it going to be used by x86 or taken advantage in common code? Linux ChangeId:ce793486e23e: driver core / ACPI: Represent ACPI companions using fwnode_handle Signed-off-by: Sameer Goel--- xen/include/asm-arm/device.h | 2 ++ xen/include/xen/fwnode.h | 33 + 2 files changed, 35 insertions(+) create mode 100644 xen/include/xen/fwnode.h diff --git a/xen/include/asm-arm/device.h b/xen/include/asm-arm/device.h index 6734ae8..78c38fe 100644 --- a/xen/include/asm-arm/device.h +++ b/xen/include/asm-arm/device.h @@ -2,6 +2,7 @@ #define __ASM_ARM_DEVICE_H #include +#include enum device_type { @@ -19,6 +20,7 @@ struct device #ifdef CONFIG_HAS_DEVICE_TREE struct dt_device_node *of_node; /* Used by drivers imported from Linux */ I was expecting a todo in the code after the discussion about leave of_node here. #endif + struct fwnode_handle *fwnode; /*fw device node identifier */ The fwnode handle was provide a match cookie for the SMMUs and not much else. Even with this around we will need the dt info in the device node. I agree that this rolls up into fw spec and I can look at the code cleanup for the next patch. A clean-up patch would be great but not necessary. What I expect is a TODO mentioning the possible clean-up. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5 11/13] xen/pvcalls: implement poll command
On 10/23/2017 07:06 PM, Stefano Stabellini wrote: > On Tue, 17 Oct 2017, Boris Ostrovsky wrote: >>> +static unsigned int pvcalls_front_poll_passive(struct file *file, >>> + struct pvcalls_bedata *bedata, >>> + struct sock_mapping *map, >>> + poll_table *wait) >>> +{ >>> + int notify, req_id, ret; >>> + struct xen_pvcalls_request *req; >>> + >>> + if (test_bit(PVCALLS_FLAG_ACCEPT_INFLIGHT, >>> +(void *)>passive.flags)) { >>> + uint32_t req_id = READ_ONCE(map->passive.inflight_req_id); >>> + >>> + if (req_id != PVCALLS_INVALID_ID && >>> + READ_ONCE(bedata->rsp[req_id].req_id) == req_id) >>> + return POLLIN | POLLRDNORM; >> >> Same READ_ONCE() question as for an earlier patch. > Same answer :-) Reviewed-by: Boris Ostrovsky> > >>> + >>> + poll_wait(file, >passive.inflight_accept_req, wait); >>> + return 0; >>> + } >>> + ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC v2] Add SUPPORT.md
On Tue, Sep 12, 2017 at 4:35 PM, Rich Persaudwrote: >> On Sep 11, 2017, at 13:01, George Dunlap wrote: >> >> +### XSM & FLASK >> + >> +Status: Experimental >> + >> +Compile time disabled >> + >> +### XSM & FLASK support for IS_PRIV >> + >> +Status: Experimental > > In which specific areas is XSM lacking in Functional completeness, Functional > stability and/or Interface stability, resulting in "Experimental" status? > What changes to XSM would be needed for it to qualify for "Supported" status? So first of all, I guess there's two "features" here: One is XSM / FLASK itself, which downstreams such OpenXT can use do make their own policies. The second is the "default FLASK policy", shipped with Xen, which has rules and labels for things in a "normal" Xen system: domUs, driver domains, stub domains, dom0, There was a time when you could simply enable that and a basic Xen System would Just Work, and (in theory) would be more secure than the default Xen system. It probably makes sense to treat these separately. Two problems we have so far: The first is that the policy bitrots fairly quickly. At the moment we don't have proper testing, and we don't really have anyone that knows how to fix it if it does break. The second problem is that while functional testing can show that the default policy is *at least* as permissive as not having FLASK enabled at all, it's a lot more difficult to show that having FLASK enabled isn't in some cases *more permissive* than we would like to be by default. We've noticed issues before where enabling XSM accidentally gives a domU access to hypercalls or settings it wouldn't have access to otherwise. Absent some way of automatically catching these changes, we're not sure we could recommend people use the default policy, even if we had confidence (via testing) that it wouldn't break people's functionality on update. The "default policy bitrot" problem won't be one for you, because (as I understand it) you write your own custom policies. But the second issue should be more concerning: when you update to a new version of Xen, what confidence do you have that your old policies will still adequately restrict guests from dangerous new functionality? I think sorting the second question out is basically what it would take to call FLASK by itself (as opposed to the default policy) "Supported". (And if you can make an argument that this is already sorted, then we can list FLASK itself as "supported".) > If there will be no security support for features in Experimental status, > would Xen Project accept patches to fix XSM security issues? Could > downstream projects issue CVEs for XSM security issues, if these will not be > issued by Xen Project? Experimental status is about 1) our assessment of how reliable the feature is, and 2) whether we will issue XSAs if security-related bugs are found. We will of course accept patches to improve functionality, and it's likely that if someone only *reports* a bug that people on the list will be able to come up with a fix. Regarding CVEs, I guess what you care about is whether as our own CNA, the XenProject would be willing to issue CVEs for XSM security issues, and/or perhaps whether we would mind if you asked Mitre directly instead. That's slightly a different topic, which we should probably discuss when we become a CNA. But to give you an idea where I'm at, I think the question is: What kind of a bug do you think you'd issue a CVE for (and/or, an XSA)? -George ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] Xen Security Advisory 236 (CVE-2017-15597) - pin count / page reference race in grant table code
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 Xen Security Advisory CVE-2017-15597 / XSA-236 version 3 pin count / page reference race in grant table code UPDATES IN VERSION 3 We now once again think that only Xen 4.2 and newer are vulnerable. Fix grammar typo. Public release. ISSUE DESCRIPTION = Grant copying code made an implication that any grant pin would be accompanied by a suitable page reference. Other portions of code, however, did not match up with that assumption. When such a grant copy operation is being done on a grant of a dying domain, the assumption turns out wrong. IMPACT == A malicious guest administrator can cause hypervisor memory corruption, most likely resulting in host crash and a Denial of Service. Privilege escalation and information leaks cannot be ruled out. VULNERABLE SYSTEMS == Xen versions from 4.2 onwards are vulnerable. Xen versions 4.1 and earlier are not vulnerable. Both x86 and ARM are vulnerable, and on x86 both PV and HVM guests can trigger the vulnerability. MITIGATION == Running only guests without para-virtual drivers, and known not to issue grant table operations can avoid the vulnerability. CREDITS === This issue was discovered by Pawel Wieczorkiewicz of Amazon. RESOLUTION == Applying the appropriate attached patch resolves this issue. xsa236.patch xen-unstable xsa236-4.9.patch Xen 4.9.x, Xen 4.8.x, Xen 4.7.x, Xen 4.6.x xsa236-4.5.patch Xen 4.5.x $ sha256sum xsa236* 2f7736c43b6da7d983cf3edbc10024c4cba9d6d3e5b2b758a07de726a804617d xsa236.meta f06f01fb4ffcfc7938a2fc6ab73559ebbaac2d448bd36ca538bb07ba510eeb4a xsa236.patch c98a4b50d021414626cd68002643e9aa0cc6067b98cd5dd995c0140a7933d1ea xsa236-4.5.patch b6fe5604af26e93184f30127ebbb644f127ecc7116b093c161ca3044b44d2fe9 xsa236-4.9.patch $ DEPLOYMENT DURING EMBARGO = Deployment of the patches and/or mitigations described above (or others which are substantially similar) is permitted during the embargo, even on public-facing systems with untrusted guest users and administrators. But: Distribution of updated software is prohibited (except to other members of the predisclosure list). Predisclosure list members who wish to deploy significantly different patches and/or mitigations, please contact the Xen Project Security Team. (Note: this during-embargo deployment notice is retained in post-embargo publicly released Xen Project advisories, even though it is then no longer applicable. This is to enable the community to have oversight of the Xen Project Security Team's decisionmaking.) For more information about permissible uses of embargoed information, consult the Xen Project community's agreed Security Policy: http://www.xenproject.org/security-policy.html -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQEcBAEBCAAGBQJZ70ZiAAoJEIP+FMlX6CvZlBgH/0cwYrP3/zvc3dNJRtpxyn1J BkigYP8JBIYW85M7KdZDFBhgXIpuw6x45XZ4qfq6rrz3GOp5oZgZVFIoggHZBzRe eVCIpjOAXInM7ThsE6pV1Qr/JKe8V6RJumXEgqr5zznWpGmcFChWmobA+BBq64P6 87ALWjXBcuqOyjJnJQwEjk+kHJMnIpocVZk6NqcDeoHoJvRh/Zk4YYc78qm4Lucw d0yHq5azA9bgt5iJgxUvF74B4r8JxTLmA8sn7Kx280UJGEAkqM7jj1QVQ6sb8fgO q6RSzBVnuVqLh4E1Dji9KaxcRRVnbrp2FFpBUUWHAVVO4O0GYlu5NxERnnye9v0= =zI77 -END PGP SIGNATURE- xsa236.meta Description: Binary data xsa236.patch Description: Binary data xsa236-4.5.patch Description: Binary data xsa236-4.9.patch Description: Binary data ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5 08/13] xen/pvcalls: implement accept command
On 10/23/2017 07:03 PM, Stefano Stabellini wrote: > On Tue, 17 Oct 2017, Boris Ostrovsky wrote: >> On 10/06/2017 08:30 PM, Stefano Stabellini wrote: >>> + /* >>> +* Backend only supports 1 inflight accept request, will return >>> +* errors for the others >>> +*/ >>> + if (test_and_set_bit(PVCALLS_FLAG_ACCEPT_INFLIGHT, >>> +(void *)>passive.flags)) { >>> + req_id = READ_ONCE(map->passive.inflight_req_id); >>> + if (req_id != PVCALLS_INVALID_ID && >>> + READ_ONCE(bedata->rsp[req_id].req_id) == req_id) { >> >> READ_ONCE (especially the second one)? I know I may sound fixated on >> this but I really don't understand how compiler may do anything wrong if >> straight reads were used. >> >> For the first case, I guess, theoretically the compiler may decide to >> re-fetch map->passive.inflight_req_id. But even if it did, would that be >> a problem? Both of these READ_ONCE targets are updated below before >> PVCALLS_FLAG_ACCEPT_INFLIGHT is cleared so there should not be any >> change between re-fetching, I think. (The only exception is the noblock >> case, which does WRITE_ONCE that don't understand either) > READ_ONCE is reasonably cheap: do we really want to have this kind of > conversation every time we touch this code in the future? Personally, I > would have used READ/WRITE_ONCE everywhere for inflight_req_id and > req_id, because it makes the code easier to understand. I guess it's a matter of opinion. I actually think it's harder to read. But it doesn't make the code wrong so... > > We have already limited their usage, but at least we have followed a set > of guidelines. Doing further optimizations on this code seems > unnecessary and prone to confuse the reader. > > >>> + ret = create_active(map2, ); >>> + if (ret < 0) { >>> + kfree(map2); >>> + clear_bit(PVCALLS_FLAG_ACCEPT_INFLIGHT, >>> + (void *)>passive.flags); >>> + spin_unlock(>socket_lock); >>> + pvcalls_exit(); >>> + return -ENOMEM; >> Why not ret? > yes, good idea. With that fixed (and extra space removed in 'ret = create_active(map2, );') Reviewed-by: Boris Ostrovsky___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] scripts: introduce a script for build test
Anthony PERARD writes ("Re: [PATCH v2] scripts: introduce a script for build test"): > That feels wrong. How do I run the same exact command at the default > one, but with -j8 instead of -j4? .../build-test sh -ec make -j4 distclean && ./configure && make -j4 But I think Anthony has a point. The clean should 1. be git-clean, not make distclean 2. be run anyway. > > +echo "Restoring original HEAD" > > +git checkout $ORIG_BRANCH > > Also, what a developper should do when the build fail? She can't modify > the current code, because changes are going to be losts. Maybe we could > trap failures, restore original HEAD and point out which commit fails to > build. IWBNI it would at least print the branch to checkout. Tools like "git bisect" record the information in .git and allow "git bisect reset". Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 01/13] x86/paravirt: remove wbinvd() paravirt interface
On 04/10/17 17:58, Josh Poimboeuf wrote: > Since lguest was removed, only the native version of wbinvd() is used. > The paravirt interface is no longer needed. > > Signed-off-by: Josh PoimboeufReviewed-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [qemu-mainline test] 115175: regressions - FAIL
flight 115175 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/115175/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-i386-xsm6 xen-buildfail REGR. vs. 114507 build-i3866 xen-buildfail REGR. vs. 114507 build-amd64-xsm 6 xen-buildfail REGR. vs. 114507 build-amd64 6 xen-buildfail REGR. vs. 114507 build-armhf-xsm 6 xen-buildfail REGR. vs. 114507 build-armhf 6 xen-buildfail REGR. vs. 114507 Tests which did not succeed, but are not blocking: test-amd64-i386-libvirt-qcow2 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64 1 build-check(1)blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-intel 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win10-i386 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-amd64-pair 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit2 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win10-i386 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-amd64-pygrub 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qcow2 1 build-check(1) blocked n/a test-amd64-amd64-amd64-pvgrub 1 build-check(1) blocked n/a test-armhf-armhf-xl-arndale 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-xl-credit2 1 build-check(1) blocked n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-i386-xl-xsm1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 1 build-check(1)blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-raw1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-amd64-i386-pvgrub 1 build-check(1) blocked n/a build-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvhv2-intel 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-xsm 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-intel 1 build-check(1) blocked n/a test-armhf-armhf-xl-vhd 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl 1 build-check(1) blocked n/a test-amd64-i386-pair 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvhv2-amd 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-amd 1 build-check(1) blocked n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) blocked n/a test-armhf-armhf-xl-rtds 1
Re: [Xen-devel] [PATCH RFC] ARM: vPL011: use receive timeout interrupt
Hi, On 23 October 2017 at 21:31, Andre Przywarawrote: > Hi, > > On 18/10/17 17:32, Bhupinder Thakur wrote: >> Hi Andre, >> >> I verified this patch on qualcomm platform. It is working fine. >> >> On 18 October 2017 at 19:11, Andre Przywara wrote: >>> Instead of asserting the receive interrupt (RXI) on the first character >>> in the FIFO, lets (ab)use the receive timeout interrupt (RTI) for that >>> purpose. That seems to be closer to the spec and what hardware does. >>> Improve the readability of vpl011_data_avail() on the way. >>> >>> Signed-off-by: Andre Przywara >>> --- >>> Hi, >>> >>> this one is the approach I mentioned in the email earlier today. >>> It goes on top of Bhupinders v12 27/27, but should eventually be merged >>> into this one once we agreed on the subject. I just carved it out here >>> for clarity to make it clearer what has been changed. >>> Would be good if someone could test it. >>> >>> Cheers, >>> Andre. >>> xen/arch/arm/vpl011.c | 61 >>> --- >>> 1 file changed, 29 insertions(+), 32 deletions(-) >>> >>> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c >>> index adf1711571..ae18bddd81 100644 >>> --- a/xen/arch/arm/vpl011.c >>> +++ b/xen/arch/arm/vpl011.c >>> @@ -105,9 +105,13 @@ static uint8_t vpl011_read_data(struct domain *d) >>> if ( fifo_level == 0 ) >>> { >>> vpl011->uartfr |= RXFE; >>> -vpl011->uartris &= ~RXI; >>> -vpl011_update_interrupt_status(d); >>> +vpl011->uartris &= ~RTI; >>> } >>> + >>> +if ( fifo_level < sizeof(intf->in) - SBSA_UART_FIFO_SIZE / 2 ) >>> +vpl011->uartris &= ~RXI; >>> + >>> +vpl011_update_interrupt_status(d); >> I think we check if ( fifo_level < SBSA_UART_FIFO_SIZE / 2 ) which >> should be a valid condition to clear the RX interrupt. > > Are you sure? My understanding is that the semantics of the return value > of xencons_queued() differs between intf and outf: > - For intf, Xen fills that buffer with incoming characters. The > watermark is assumed to be (FIFO / 2), which translates into 16 > characters. Now for the SBSA vUART RX side that means: "Assert the RX > interrupt if there is only room for 16 (or less) characters in the FIFO > (read: intf buffer in our case). Since we (ab)use the Xen buffer for the > FIFO, this means we warn if the number of queued characters exceeds > (buffersize - 16). > - For outf, the UART emulation fills the buffer. The SBSA vUART TX side > demands that the TX interrupt is asserted if the fill level of the > transmit FIFO is less than or equal to the 16 characters, which means: > number of queued characters is less than 16. > > I think the key point is that our trigger level isn't symmetrical here, > since we have to emulate the architected 32-byte FIFO semantics for the > driver, but have a (secretly) much larger "FIFO" internally. > > Do you agree with this reasoning and do I have a thinko here? Could well > be I am seriously misguided here. > ok. I agree with the description as it will expose the same behavior to the driver as it would be there for a real UART where only FIFO/2 space is left. Regards, Bhupinder ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC] ARM: vPL011: use receive timeout interrupt
Hi Andre, On 24 October 2017 at 16:57, Andre Przywarawrote: > Hi, > > On 24/10/17 12:00, Julien Grall wrote: >> Hi, >> >> On 23/10/2017 17:01, Andre Przywara wrote: >>> Hi, >>> >>> On 18/10/17 17:32, Bhupinder Thakur wrote: Hi Andre, I verified this patch on qualcomm platform. It is working fine. On 18 October 2017 at 19:11, Andre Przywara wrote: > Instead of asserting the receive interrupt (RXI) on the first character > in the FIFO, lets (ab)use the receive timeout interrupt (RTI) for that > purpose. That seems to be closer to the spec and what hardware does. > Improve the readability of vpl011_data_avail() on the way. > > Signed-off-by: Andre Przywara > --- > Hi, > > this one is the approach I mentioned in the email earlier today. > It goes on top of Bhupinders v12 27/27, but should eventually be merged > into this one once we agreed on the subject. I just carved it out here > for clarity to make it clearer what has been changed. > Would be good if someone could test it. > > Cheers, > Andre. > xen/arch/arm/vpl011.c | 61 > --- > 1 file changed, 29 insertions(+), 32 deletions(-) > > diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c > index adf1711571..ae18bddd81 100644 > --- a/xen/arch/arm/vpl011.c > +++ b/xen/arch/arm/vpl011.c > @@ -105,9 +105,13 @@ static uint8_t vpl011_read_data(struct domain *d) > if ( fifo_level == 0 ) > { > vpl011->uartfr |= RXFE; > -vpl011->uartris &= ~RXI; > -vpl011_update_interrupt_status(d); > +vpl011->uartris &= ~RTI; > } > + > +if ( fifo_level < sizeof(intf->in) - SBSA_UART_FIFO_SIZE / 2 ) > +vpl011->uartris &= ~RXI; > + > +vpl011_update_interrupt_status(d); I think we check if ( fifo_level < SBSA_UART_FIFO_SIZE / 2 ) which should be a valid condition to clear the RX interrupt. >>> >>> Are you sure? My understanding is that the semantics of the return value >>> of xencons_queued() differs between intf and outf: >>> - For intf, Xen fills that buffer with incoming characters. The >>> watermark is assumed to be (FIFO / 2), which translates into 16 >>> characters. Now for the SBSA vUART RX side that means: "Assert the RX >>> interrupt if there is only room for 16 (or less) characters in the FIFO >>> (read: intf buffer in our case). Since we (ab)use the Xen buffer for the >>> FIFO, this means we warn if the number of queued characters exceeds >>> (buffersize - 16). >>> - For outf, the UART emulation fills the buffer. The SBSA vUART TX side >>> demands that the TX interrupt is asserted if the fill level of the >>> transmit FIFO is less than or equal to the 16 characters, which means: >>> number of queued characters is less than 16. >>> >>> I think the key point is that our trigger level isn't symmetrical here, >>> since we have to emulate the architected 32-byte FIFO semantics for the >>> driver, but have a (secretly) much larger "FIFO" internally. >>> >>> Do you agree with this reasoning and do I have a thinko here? Could well >>> be I am seriously misguided here. >> >> xencons_queued calculates how many bytes are currently on the ring. So I >> think your description makes sense. >> >> With (fifo_level < (SBSA_UART_FIFO_SIZE / 2)), you would only clear it >> when the ring has less than 16 bytes queued. >> >> I have a few requests on those patches for the resender: >> - Please introduce a define for SBSA_UART_FIFO_SIZE / 2 and use it >> everywhere. >> - Please add a bit more documentation on top of the checks in >> vpl011_read_data function. The checks in vpl011_write_data looks >> well-documented. > > I am just at rewording the commit message and was planning on re-sending > the (merged) patches later today (keeping Bhupinder's authorship, of > course). > > I hope that Bhupinder doesn't mind or this doesn't clash with any of his > plans. It is fine with me. Thanks. Regards, Bhupinder ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [libvirt test] 115172: trouble: broken/pass
flight 115172 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/115172/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-libvirt broken test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm broken test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 4 host-install(4) broken REGR. vs. 114825 test-amd64-amd64-libvirt 4 host-install(4)broken REGR. vs. 114825 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail like 114825 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 114825 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 114825 test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qcow2 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass version targeted for testing: libvirt 55ac6a5d44a2fbd7e93d995930d6071030c907ba baseline version: libvirt 08d4e16f88f9cb0e078b544f49a0647c8847fe95 Last test of basis 114825 2017-10-21 04:47:31 Z3 days Testing same since 115172 2017-10-24 04:20:11 Z0 days1 attempts People who touched revisions under test: Jiri DenemarkMichal Privoznik jobs: build-amd64-xsm pass build-armhf-xsm pass build-i386-xsm pass build-amd64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmbroken test-amd64-amd64-libvirt-xsm pass test-armhf-armhf-libvirt-xsm pass test-amd64-i386-libvirt-xsm pass test-amd64-amd64-libvirt broken test-armhf-armhf-libvirt pass test-amd64-i386-libvirt pass test-amd64-amd64-libvirt-pairpass test-amd64-i386-libvirt-pair pass test-amd64-i386-libvirt-qcow2pass test-armhf-armhf-libvirt-raw pass test-amd64-amd64-libvirt-vhd pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary broken-job test-amd64-amd64-libvirt broken broken-job test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm broken broken-step test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm host-install(4) broken-step test-amd64-amd64-libvirt host-install(4) Not pushing. commit 55ac6a5d44a2fbd7e93d995930d6071030c907ba Author: Jiri Denemark Date: Thu Oct 19 14:26:24 2017 +0200 qemu: Set correct job status when qemuMigrationRun fails Instead of enumerating all states which need to be turned into
Re: [Xen-devel] [PATCH v9.1 02/16] Rename PSR sysctl/domctl interfaces and xsm policy to make them be general
>>> On 24.10.17 at 11:10,wrote: > This patch renames PSR sysctl/domctl interfaces and related xsm policy to > make them be general for all resource allocation features but not only > for CAT. Then, we can resuse the interfaces for all allocation features. > > Basically, it changes 'psr_cat_op' to 'psr_alloc', and remove 'CAT_' from some > macros. E.g.: > 1. psr_cat_op -> psr_alloc > 2. XEN_DOMCTL_psr_cat_op -> XEN_DOMCTL_psr_alloc > 3. XEN_SYSCTL_psr_cat_op -> XEN_SYSCTL_psr_alloc > 4. XEN_DOMCTL_PSR_CAT_SET_L3_CBM -> XEN_DOMCTL_PSR_SET_L3_CBM > 5. XEN_SYSCTL_PSR_CAT_get_l3_info -> XEN_SYSCTL_PSR_get_l3_info > > Signed-off-by: Yi Sun > Reviewed-by: Wei Liu > Reviewed-by: Roger Pau Monné > Acked-by: Jan Beulich > Acked-by: Daniel De Graaf > --- > CC: Jan Beulich > CC: Andrew Cooper > CC: Wei Liu > CC: Ian Jackson > CC: Daniel De Graaf > CC: Roger Pau Monné > CC: Chao Peng > > v9: > - rename 'psr_cat_op' to 'psr_alloc' in xen.if. Even if this was just a simple oversight and an easy rename, I think strictly speaking it invalidates Daniel's ack. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] Is that possible to merge MBA into Xen 4.10?
>>> On 24.10.17 at 14:14,wrote: > a) Sticking to the rules > I think in some cases where a few days have been missed, we should have > enough flexibility to bend the rules. > In fact, if say a crucial part of PVHv2 missed the deadline by a few days, > we would probably bend the rules. For a crucial series, I might agree. But allowing " a few days" to me then puts under question what the deadline is for. > Of course whether something is a "niche" feature is in view of the beholder. Sure. > c) PR Perspective > I am somewhat concerned that we do not have a lot of stuff for good media > coverage for Xen 4.10 > > This is a relatively small release: from what I can see we have > approximately only around 235 patch series (4.9 had 481, 4.8 had 575) > Admittedly the number of patches in 4.10 is quite high: approx 1707 patches > (with 1549 in 4.9 and 1245 in 4.8) > A lot of it seems to be groundwork for 4.11 (or even 4.12) > > Besides PVHv2, we don't have a lot of big marketable new stuff which would > enable us to get press quotes. > Admittedly, I have not put a list of marketable features together yet. > A feature such as MBA, would help from a PR perspective. I'm sorry, but this is a non-argument to me. Most everyone besides me wanted shorter release cycles. Possibly having fewer PR relevant features is a direct result of that. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] Is that possible to merge MBA into Xen 4.10?
Hi all, > On 24 Oct 2017, at 09:35, Jan Beulichwrote: > On 24.10.17 at 04:10, wrote: >> As you may know, MBA patch set has got enough Reviewed-by/Acked-by in last >> week. >> It is ready to be merged. >> >> This is a feature for Skylake, Intel has launched Skylake and KVM already >> supported MBA, so including it in Xen 4.10 will quickly fill this gap. >> >> MBA missed the 4.10 feature freeze date for only a few days due to lack of >> timely review for earlier versions which slowed down the patch iteration >> notably. >> It seems maintainers are very busy recently so that the review progress for >> 4.10 >> is slower than before. So I am wondering if it is possible to merge it into >> 4.10? >> >> This patch set mainly touches codes related to PSR in >> tools/domctl/sysctl/hypervisor. >> It does not touch other features. So, the risk is low to merge it. > > While I agree the risk is low, I think we should not start making > exceptions from the "no freeze exceptions" rule. Even less so > for a secondary (or should I pull out my favorite "niche" again?) > feature like this one. But in the end it's Julien's call., of course. I think there are a number of separate issues concerns, which are somewhat orthogonal: a) Sticking to the rules I think in some cases where a few days have been missed, we should have enough flexibility to bend the rules. In fact, if say a crucial part of PVHv2 missed the deadline by a few days, we would probably bend the rules. Of course whether something is a "niche" feature is in view of the beholder. b) Risk This is something which is clearly important. c) PR Perspective I am somewhat concerned that we do not have a lot of stuff for good media coverage for Xen 4.10 This is a relatively small release: from what I can see we have approximately only around 235 patch series (4.9 had 481, 4.8 had 575) Admittedly the number of patches in 4.10 is quite high: approx 1707 patches (with 1549 in 4.9 and 1245 in 4.8) A lot of it seems to be groundwork for 4.11 (or even 4.12) Besides PVHv2, we don't have a lot of big marketable new stuff which would enable us to get press quotes. Admittedly, I have not put a list of marketable features together yet. A feature such as MBA, would help from a PR perspective. But ultimately this is going to be Julien's call. Regards Lars ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] migration, xen: Fix block image lock issue on live migration
On Wed, Oct 04, 2017 at 03:03:49PM +0200, Kevin Wolf wrote: > Am 02.10.2017 um 21:18 hat Dr. David Alan Gilbert geschrieben: > > Adding in kwolf; it looks sane to me; Kevin? > > If I'm reading this right, this is just after the device state save. > > Is this actual migration? Because the code looks more like it's copied > and adapted from the snapshot code rather than from the actual migration > code. Well the Xen tool stack takes care of the migration, we only need to save the device states from QEMU, I guess similair to a snapshot. > If Xen doesn't use the standard mechanisms, I don't know what they need > to do. Snapshots don't need to inactivate images, but migration does. > Compared to the normal migration path, this looks very simplistic, so I > wouldn't be surprised if there was more wrong than just file locking. I realize now that if one would want to take a snapshot of a running Xen guest, this xen-save-devices-state qmp command will be called as well. So I can see a few options to better handle snapshots, we could: - Add a new parameter to xen-save-devices-state, "live_migration" which could default to 'true' so older version of Xen will still works. - Create a new qmp command that sole purpose is to call bdrv_inactivate_all, I don't know what else this command would have to do. - or just take this patch. Thanks. > This looks like it could work as a hack to the problem at hand. Whether > it is a proper solution, I can't say without investing a lot more time. -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [linux-linus test] 115170: regressions - trouble: broken/fail/pass
flight 115170 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/115170/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-libvirt-pair broken test-amd64-i386-pair broken test-amd64-amd64-libvirt-pair 4 host-install/src_host(4) broken REGR. vs. 114682 test-amd64-i386-pair4 host-install/src_host(4) broken REGR. vs. 114682 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail REGR. vs. 114682 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 114682 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 114682 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 114682 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 114682 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 114682 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 114682 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail like 114682 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 114682 test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-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-i386-libvirt-qcow2 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-xsm 13 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-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass version targeted for testing: linux6cff0a118f23b98c604a3604ea9de11338e24fbe baseline version: linuxebe6e90ccc6679cb01d2b280e4b61e6092d4bedb Last test of basis 114682 2017-10-18 09:54:11 Z6 days Failing since114781 2017-10-20 01:00:47 Z4 days8 attempts Testing same since 115170 2017-10-24 02:47:58 Z0 days1 attempts 316 people touched revisions under test, not listing them all jobs: build-amd64-xsm pass build-armhf-xsm pass build-i386-xsm pass build-amd64 pass build-armhf pass build-i386 pass build-amd64-libvirt
Re: [Xen-devel] [PATCH RFC v2] Add SUPPORT.md
On 24/10/17 11:27, George Dunlap wrote: > On 10/23/2017 06:55 PM, Andrew Cooper wrote: >> On 23/10/17 17:22, George Dunlap wrote: >>> On 09/11/2017 06:53 PM, Andrew Cooper wrote: On 11/09/17 18:01, George Dunlap wrote: > +### x86/RAM > + > +Limit, x86: 16TiB > +Limit, ARM32: 16GiB > +Limit, ARM64: 5TiB > + > +[XXX: Andy to suggest what this should say for x86] The limit for x86 is either 16TiB or 123TiB, depending on CONFIG_BIGMEM. CONFIG_BIGMEM is exposed via menuconfig without XEN_CONFIG_EXPERT, so falls into at least some kind of support statement. As for practical limits, I don't think its reasonable to claim anything which we can't test. What are the specs in the MA colo? >>> At the moment the "Limit" tag specifically says that it's theoretical >>> and may not work. >>> >>> We could add another tag, "Limit-tested", or something like that. >>> >>> Or, we could simply have the Limit-security be equal to the highest >>> amount which has been tested (either by osstest or downstreams). >>> >>> For simplicity's sake I'd go with the second one. >> It think it would be very helpful to distinguish the upper limits from >> the supported limits. There will be a large difference between the two. >> >> Limit-Theoretical and Limit-Supported ? > Well "supported" without any modifiers implies "security supported". So > perhaps we could just `s/Limit-security/Limit-supported/;` ? By this, you mean use Limit-Supported throughout this document? That sounds like a good plan. > > +Limit, x86 HVM: 128 > +Limit, ARM32: 8 > +Limit, ARM64: 128 > + > +[XXX Andrew Cooper: Do want to add "Limit-Security" here for some of > these?] 32 for each. 64 vcpu HVM guests can excerpt enough p2m lock pressure to trigger a 5 second host watchdog timeout. >>> Is that "32 for x86 PV and x86 HVM", or "32 for x86 HVM and ARM64"? Or >>> something else? >> The former. I'm not qualified to comment on any of the ARM limits. >> >> There are several non-trivial for_each_vcpu() loops in the domain_kill >> path which aren't handled by continuations. ISTR 128 vcpus is enough to >> trip a watchdog timeout when freeing pagetables. > I don't think 32 is a really practical limit. What do you mean by practical here, and what evidence are you basing this on? Amongst other things, there is an ABI boundary in Xen at 32 vcpus, and given how often it is broken in Linux, its clear that there isn't regular testing happening beyond this limit. > I'm inclined to say that if a rogue guest can crash a host with 33 vcpus, we > should issue an XSA > and fix it. The reason XenServer limits at 32 vcpus is that I can crash Xen with a 64 vcpu HVM domain. The reason it hasn't been my top priority to fix this is because there is very little customer interest in pushing this limit higher. Obviously, we should fix issues as and when they are discovered, and work towards increasing the limits in the longterm, but saying "this limit seems too low, so lets provisionally set it higher" is short sighted and a recipe for more XSAs. > + > +### x86 PV/Event Channels > + > +Limit: 131072 Why do we call out event channel limits but not grant table limits? Also, why is this x86? The 2l and fifo ABIs are arch agnostic, as far as I am aware. >>> Sure, but I'm pretty sure that ARM guests don't (perhaps cannot?) use PV >>> event channels. >> This is mixing the hypervisor API/ABI capabilities with the actual >> abilities of guests (which is also different to what Linux would use in >> the guests). > I'd say rather that you are mixing up the technical abilities of a > system with user-facing features. :-) At the moment there is no reason > for any ARM user to even think about event channels, so there's no > reason to bother them with the technical details. If at some point that > changes, we can modify the document. You do realise that receiving an event is entirely asymmetric with sending an event? Even on ARM, {net,blk}front needs to speak event_{2l,fifo} with Xen to bind and use its interdomain event channel(s) with {net,blk}back. > >> ARM guests, as well as x86 HVM with APICV (configured properly) will >> actively want to avoid the guest event channel interface, because its >> slower. >> >> This solitary evtchn limit serves no useful purpose IMO. > There may be a point to what you're saying: The event channel limit > normally manifests itself as a limit on the number of guests / total > devices. > > On the other hand, having these kinds of limits around does make sense. > > Let me give it some thoughts. (If anyone else has any opinions...) The event_fifo limit is per-domain, not system-wide. In general this only matters for a monolithic dom0, as it is one end of each event channel in the system. > > +## High Availability and Fault Tolerance > + > +### Live Migration, Save & Restore > +
Re: [Xen-devel] [PATCH v2] scripts: introduce a script for build test
On Mon, Oct 23, 2017 at 05:56:33PM +0100, Wei Liu wrote: > + > +if test $# -lt 2 ; then > +echo "Usage: $0 [CMD]" > +exit 1 > +fi [...] > +git rev-list $BASE..$TIP | nl -ba | tac | \ > +while read num rev; do > +echo "Testing $num $rev" > +git checkout $rev > +if test $# -eq 0 ; then > +make -j4 distclean && ./configure && make -j4 > +else > +"$@" That feels wrong. How do I run the same exact command at the default one, but with -j8 instead of -j4? I can see only two options, but I'm not sure if it is what you had in mind: - Option #1: a script $ echo 'make -j8 distclean && ./configure && make -j8' > tmp-script.sh $ ./script/build-test.sh master my-feature bash tmp-script.sh - Option #2: with eval! $ ./script/build-test.sh master my-feature eval make -j8 distclean '&&' ./configure '&&' make -j8 # notice the eval ... here :-) > +fi > +echo > +done > + > +echo "Restoring original HEAD" > +git checkout $ORIG_BRANCH Also, what a developper should do when the build fail? She can't modify the current code, because changes are going to be losts. Maybe we could trap failures, restore original HEAD and point out which commit fails to build. Another thing that can be done is do the build test in a temporary checkout, but I'm not sure if it is a good idea. (I'm still trying to find out how a script can do a better job than a plain `git rebase --interactive --exec 'blah'`, it is maybe just because I know what to do if there is an issue.) -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC] ARM: vPL011: use receive timeout interrupt
Hi, On 24/10/17 12:00, Julien Grall wrote: > Hi, > > On 23/10/2017 17:01, Andre Przywara wrote: >> Hi, >> >> On 18/10/17 17:32, Bhupinder Thakur wrote: >>> Hi Andre, >>> >>> I verified this patch on qualcomm platform. It is working fine. >>> >>> On 18 October 2017 at 19:11, Andre Przywara>>> wrote: Instead of asserting the receive interrupt (RXI) on the first character in the FIFO, lets (ab)use the receive timeout interrupt (RTI) for that purpose. That seems to be closer to the spec and what hardware does. Improve the readability of vpl011_data_avail() on the way. Signed-off-by: Andre Przywara --- Hi, this one is the approach I mentioned in the email earlier today. It goes on top of Bhupinders v12 27/27, but should eventually be merged into this one once we agreed on the subject. I just carved it out here for clarity to make it clearer what has been changed. Would be good if someone could test it. Cheers, Andre. xen/arch/arm/vpl011.c | 61 --- 1 file changed, 29 insertions(+), 32 deletions(-) diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c index adf1711571..ae18bddd81 100644 --- a/xen/arch/arm/vpl011.c +++ b/xen/arch/arm/vpl011.c @@ -105,9 +105,13 @@ static uint8_t vpl011_read_data(struct domain *d) if ( fifo_level == 0 ) { vpl011->uartfr |= RXFE; - vpl011->uartris &= ~RXI; - vpl011_update_interrupt_status(d); + vpl011->uartris &= ~RTI; } + + if ( fifo_level < sizeof(intf->in) - SBSA_UART_FIFO_SIZE / 2 ) + vpl011->uartris &= ~RXI; + + vpl011_update_interrupt_status(d); >>> I think we check if ( fifo_level < SBSA_UART_FIFO_SIZE / 2 ) which >>> should be a valid condition to clear the RX interrupt. >> >> Are you sure? My understanding is that the semantics of the return value >> of xencons_queued() differs between intf and outf: >> - For intf, Xen fills that buffer with incoming characters. The >> watermark is assumed to be (FIFO / 2), which translates into 16 >> characters. Now for the SBSA vUART RX side that means: "Assert the RX >> interrupt if there is only room for 16 (or less) characters in the FIFO >> (read: intf buffer in our case). Since we (ab)use the Xen buffer for the >> FIFO, this means we warn if the number of queued characters exceeds >> (buffersize - 16). >> - For outf, the UART emulation fills the buffer. The SBSA vUART TX side >> demands that the TX interrupt is asserted if the fill level of the >> transmit FIFO is less than or equal to the 16 characters, which means: >> number of queued characters is less than 16. >> >> I think the key point is that our trigger level isn't symmetrical here, >> since we have to emulate the architected 32-byte FIFO semantics for the >> driver, but have a (secretly) much larger "FIFO" internally. >> >> Do you agree with this reasoning and do I have a thinko here? Could well >> be I am seriously misguided here. > > xencons_queued calculates how many bytes are currently on the ring. So I > think your description makes sense. > > With (fifo_level < (SBSA_UART_FIFO_SIZE / 2)), you would only clear it > when the ring has less than 16 bytes queued. > > I have a few requests on those patches for the resender: > - Please introduce a define for SBSA_UART_FIFO_SIZE / 2 and use it > everywhere. > - Please add a bit more documentation on top of the checks in > vpl011_read_data function. The checks in vpl011_write_data looks > well-documented. I am just at rewording the commit message and was planning on re-sending the (merged) patches later today (keeping Bhupinder's authorship, of course). I hope that Bhupinder doesn't mind or this doesn't clash with any of his plans. Cheers, Andre. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 00/33] osstest: FreeBSD host support
Roger Pau Monné writes ("Re: [PATCH v12 00/33] osstest: FreeBSD host support"): > Sorry for the delay, had to cherry-pick some commits from the FreeBSD > host install series in order for the examine one to work. I've pushed > this to the following branch: > > git://xenbits.xen.org/people/royger/osstest.git examine I have now rebased this onto my small queue (of patches to admin tools which should not cause trouble) and pushed it to pretest. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC] ARM: vPL011: use receive timeout interrupt
Hi, On 23/10/2017 17:01, Andre Przywara wrote: Hi, On 18/10/17 17:32, Bhupinder Thakur wrote: Hi Andre, I verified this patch on qualcomm platform. It is working fine. On 18 October 2017 at 19:11, Andre Przywarawrote: Instead of asserting the receive interrupt (RXI) on the first character in the FIFO, lets (ab)use the receive timeout interrupt (RTI) for that purpose. That seems to be closer to the spec and what hardware does. Improve the readability of vpl011_data_avail() on the way. Signed-off-by: Andre Przywara --- Hi, this one is the approach I mentioned in the email earlier today. It goes on top of Bhupinders v12 27/27, but should eventually be merged into this one once we agreed on the subject. I just carved it out here for clarity to make it clearer what has been changed. Would be good if someone could test it. Cheers, Andre. xen/arch/arm/vpl011.c | 61 --- 1 file changed, 29 insertions(+), 32 deletions(-) diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c index adf1711571..ae18bddd81 100644 --- a/xen/arch/arm/vpl011.c +++ b/xen/arch/arm/vpl011.c @@ -105,9 +105,13 @@ static uint8_t vpl011_read_data(struct domain *d) if ( fifo_level == 0 ) { vpl011->uartfr |= RXFE; -vpl011->uartris &= ~RXI; -vpl011_update_interrupt_status(d); +vpl011->uartris &= ~RTI; } + +if ( fifo_level < sizeof(intf->in) - SBSA_UART_FIFO_SIZE / 2 ) +vpl011->uartris &= ~RXI; + +vpl011_update_interrupt_status(d); I think we check if ( fifo_level < SBSA_UART_FIFO_SIZE / 2 ) which should be a valid condition to clear the RX interrupt. Are you sure? My understanding is that the semantics of the return value of xencons_queued() differs between intf and outf: - For intf, Xen fills that buffer with incoming characters. The watermark is assumed to be (FIFO / 2), which translates into 16 characters. Now for the SBSA vUART RX side that means: "Assert the RX interrupt if there is only room for 16 (or less) characters in the FIFO (read: intf buffer in our case). Since we (ab)use the Xen buffer for the FIFO, this means we warn if the number of queued characters exceeds (buffersize - 16). - For outf, the UART emulation fills the buffer. The SBSA vUART TX side demands that the TX interrupt is asserted if the fill level of the transmit FIFO is less than or equal to the 16 characters, which means: number of queued characters is less than 16. I think the key point is that our trigger level isn't symmetrical here, since we have to emulate the architected 32-byte FIFO semantics for the driver, but have a (secretly) much larger "FIFO" internally. Do you agree with this reasoning and do I have a thinko here? Could well be I am seriously misguided here. xencons_queued calculates how many bytes are currently on the ring. So I think your description makes sense. With (fifo_level < (SBSA_UART_FIFO_SIZE / 2)), you would only clear it when the ring has less than 16 bytes queued. I have a few requests on those patches for the resender: - Please introduce a define for SBSA_UART_FIFO_SIZE / 2 and use it everywhere. - Please add a bit more documentation on top of the checks in vpl011_read_data function. The checks in vpl011_write_data looks well-documented. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] mg-hosts: Fix usage of showprops
Anthony PERARD writes ("[PATCH] mg-hosts: Fix usage of showprops"): > ./mg-hosts showprops XXX description and implementation didn't match. > Fix description. Thanks, queued. (I edited the commit message a bit.) Ian. From 10ce43294150c6cfd5b00154636eb8cb945b7188 Mon Sep 17 00:00:00 2001 From: Anthony PERARDDate: Tue, 24 Oct 2017 11:37:20 +0100 Subject: [OSSTEST PATCH] mg-hosts: Fix of showprops doc comment ./mg-hosts showprops description and implementation didn't match. Fix description. Signed-off-by: Anthony PERARD Signed-off-by: Ian Jackson --- mg-hosts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mg-hosts b/mg-hosts index a000f2d..d91a965 100755 --- a/mg-hosts +++ b/mg-hosts @@ -54,8 +54,8 @@ # old tasks might still mess about with the resources, # interfering with whatever new tasks allocate them. # -# ./mg-hosts showprops [HOST...] -# Prints the resource properties of all or specified hosts. +# ./mg-hosts showprops [PROP...] +# Prints all or specified resource properties of all hosts. # # ./mg-hosts setprops HOSTGLOB... [- PROP [OLD] NEW ...] -|-- PROP [OLD] NEW # Updates resource properties of the specified hosts. -- 2.1.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] mg-hosts: Fix usage of showprops
./mg-hosts showprops XXX description and implementation didn't match. Fix description. Signed-off-by: Anthony PERARD--- mg-hosts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mg-hosts b/mg-hosts index a000f2d..d91a965 100755 --- a/mg-hosts +++ b/mg-hosts @@ -54,8 +54,8 @@ # old tasks might still mess about with the resources, # interfering with whatever new tasks allocate them. # -# ./mg-hosts showprops [HOST...] -# Prints the resource properties of all or specified hosts. +# ./mg-hosts showprops [PROP...] +# Prints all or specified resource properties of all hosts. # # ./mg-hosts setprops HOSTGLOB... [- PROP [OLD] NEW ...] -|-- PROP [OLD] NEW # Updates resource properties of the specified hosts. -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC v2] Add SUPPORT.md
Hi, On 23/10/2017 18:55, Andrew Cooper wrote: On 23/10/17 17:22, George Dunlap wrote: On 09/11/2017 06:53 PM, Andrew Cooper wrote: On 11/09/17 18:01, George Dunlap wrote: +Limit, x86 HVM: 128 +Limit, ARM32: 8 +Limit, ARM64: 128 + +[XXX Andrew Cooper: Do want to add "Limit-Security" here for some of these?] 32 for each. 64 vcpu HVM guests can excerpt enough p2m lock pressure to trigger a 5 second host watchdog timeout. Is that "32 for x86 PV and x86 HVM", or "32 for x86 HVM and ARM64"? Or something else? The former. I'm not qualified to comment on any of the ARM limits. That's a good question. On Arm32 the number of vCPUs is limited by the GICv2 implementation. On Arm64, GICv2 platform can only support up to 8 vCPUs. GICv3 is theoretically 4096. But it is capped to 128 vCPUs, IIRC it was just to match x86. There are several non-trivial for_each_vcpu() loops in the domain_kill path which aren't handled by continuations. ISTR 128 vcpus is enough to trip a watchdog timeout when freeing pagetables. On Arm, we have similar for_each_vcpu() in the vGIC code to inject SPIs (see vgic_to_sgi). I haven't tried it so far with a high number of vCPUs. So I am not sure if we should stick to 128 too. Stefano do you have any opinions? Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC v2] Add SUPPORT.md
On 10/23/2017 06:55 PM, Andrew Cooper wrote: > On 23/10/17 17:22, George Dunlap wrote: >> On 09/11/2017 06:53 PM, Andrew Cooper wrote: >>> On 11/09/17 18:01, George Dunlap wrote: +### x86/RAM + +Limit, x86: 16TiB +Limit, ARM32: 16GiB +Limit, ARM64: 5TiB + +[XXX: Andy to suggest what this should say for x86] >>> The limit for x86 is either 16TiB or 123TiB, depending on >>> CONFIG_BIGMEM. CONFIG_BIGMEM is exposed via menuconfig without >>> XEN_CONFIG_EXPERT, so falls into at least some kind of support statement. >>> >>> As for practical limits, I don't think its reasonable to claim anything >>> which we can't test. What are the specs in the MA colo? >> At the moment the "Limit" tag specifically says that it's theoretical >> and may not work. >> >> We could add another tag, "Limit-tested", or something like that. >> >> Or, we could simply have the Limit-security be equal to the highest >> amount which has been tested (either by osstest or downstreams). >> >> For simplicity's sake I'd go with the second one. > > It think it would be very helpful to distinguish the upper limits from > the supported limits. There will be a large difference between the two. > > Limit-Theoretical and Limit-Supported ? Well "supported" without any modifiers implies "security supported". So perhaps we could just `s/Limit-security/Limit-supported/;` ? > > In all cases, we should identify why the limit is where it is, even if > that is only "maximum people have tested to". Other This document is already fairly complicated, and a massive amount of work (as each line is basically an invitation to bike-shedding). If it's OK with you, I'll leave the introduction of where the limit comes from for a motivated individual to add in a subsequent patch. :-) >> Shall I write an e-mail with a more direct query for the maximum amounts >> of various numbers tested by the XenProject (via osstest), Citrix, SuSE, >> and Oracle? > > For XenServer, > http://docs.citrix.com/content/dam/docs/en-us/xenserver/current-release/downloads/xenserver-config-limits.pdf > >>> [root@fusebot ~]# python >>> Python 2.7.5 (default, Nov 20 2015, 02:00:19) >>> [GCC 4.8.5 20150623 (Red Hat 4.8.5-4)] on linux2 >>> Type "help", "copyright", "credits" or "license" for more information. >> from xen.lowlevel.xc import xc as XC >> xc = XC() >> xc.domain_create() >>> 1 >> xc.domain_max_vcpus(1, 8192) >>> 0 >> xc.domain_create() >>> 2 >> xc.domain_max_vcpus(2, 8193) >>> Traceback (most recent call last): >>> File "", line 1, in >>> xen.lowlevel.xc.Error: (22, 'Invalid argument') >>> >>> Trying to shut such a domain down however does tickle a host watchdog >>> timeout as the for_each_vcpu() loops in domain_kill() are very long. >> For now I'll set 'Limit' to 8192, and 'Limit-security' to 512. >> Depending on what I get for the "test limit" survey I may adjust it >> afterwards. > > The largest production x86 server I am aware of is a Skylake-S system > with 496 threads. 512 is not a plausibly-tested number. > >> +Limit, x86 HVM: 128 +Limit, ARM32: 8 +Limit, ARM64: 128 + +[XXX Andrew Cooper: Do want to add "Limit-Security" here for some of these?] >>> 32 for each. 64 vcpu HVM guests can excerpt enough p2m lock pressure to >>> trigger a 5 second host watchdog timeout. >> Is that "32 for x86 PV and x86 HVM", or "32 for x86 HVM and ARM64"? Or >> something else? > > The former. I'm not qualified to comment on any of the ARM limits. > > There are several non-trivial for_each_vcpu() loops in the domain_kill > path which aren't handled by continuations. ISTR 128 vcpus is enough to > trip a watchdog timeout when freeing pagetables. I don't think 32 is a really practical limit. I'm inclined to say that if a rogue guest can crash a host with 33 vcpus, we should issue an XSA and fix it. +### Virtual RAM + +Limit, x86 PV: >1TB +Limit, x86 HVM: 1TB +Limit, ARM32: 16GiB +Limit, ARM64: 1TB >>> There is no specific upper bound on the size of PV or HVM guests that I >>> am aware of. 1.5TB HVM domains definitely work, because that's what we >>> test and support in XenServer. >> Are there limits for 32-bit guests? There's some complicated limit >> having to do with the m2p, right? > > 32bit PV guests need to live in MFNs under the 128G boundary, despite > the fact their p2m handling supports 4TB of RAM. That's what I was looking for. Let me see if I can find a concise way to represent that. > The PVinPVH plan will lift this limitation, at which point it will be > possible to have many 128G 32bit PV(inPVH) VMs on a large system. > (OTOH, I'm not aware of any 32bit PV guest which itself supports more > than 64G of RAM, other than perhaps SLES 11.) Right, but PVinPVH is a different monster. + +### x86 PV/Event Channels + +Limit: 131072 >>> Why do we call out event channel limits but
[Xen-devel] [PATCH v8] x86/altp2m: support for setting restrictions for an array of pages
From: Razvan CojocaruFor the default EPT view we have xc_set_mem_access_multi(), which is able to set an array of pages to an array of access rights with a single hypercall. However, this functionality was lacking for the altp2m subsystem, which could only set page restrictions for one page at a time. This patch addresses the gap. HVMOP_altp2m_set_mem_access_multi has been added as a HVMOP (as opposed to a DOMCTL) for consistency with its HVMOP_altp2m_set_mem_access counterpart (and hence with the original altp2m design, where domains are allowed - with the proper altp2m access rights - to alter these settings), in the absence of an official position on the issue from the original altp2m designers. Signed-off-by: Razvan Cojocaru Signed-off-by: Petre Pircalabu --- Changed since v2: * Added support for compat arguments translation Changed since v3: * Replaced __copy_to_guest with __copy_field_to_guest * Removed the un-needed parentheses. * Fixed xlat.lst ordering * Added comment to patch description explaining why the functionality was added as an HVMOP. * Guard using XEN_GENERATING_COMPAT_HEADERS the hvmmem_type_t definition. This will prevent suplicate definitions to be generated for the compat equivalent. * Added comment describing the manual translation of xen_hvm_altp2m_op_t generic fields from compat_hvm_altp2m_op_t. Changed since v4: * Changed the mask parameter to 0x3F. * Split long lines. * Added "improperly named HVMMEM_(*)" to the comment explaining the XEN_GENERATING_COMPAT_HEADERS guard. * Removed typedef and XEN_GUEST_HANDLE for xen_hvm_altp2m_set_mem_access_multi. * Copied the "opaque" field to guest in compat_altp2m_op. * Added build time test to check if the size of xen_hvm_altp2m_set_mem_access_multi at least equal to the size of compat_hvm_altp2m_set_mem_access_multi. Changed since v5: * Changed the domid parameter type to uint32_t to match 5b42c82f. * Added comment to explain why the 0x3F value was chosen. * Fixed switch indentation in compat_altp2m_op. * Changed the condition used to check if the opaque field has to be copied to the guest. * Added CHECK_hvm_altp2m_op and CHECK_hvm_altp2m_set_mem_access_multi. Changed since v6: * Removed trailing semicolon from the definitions of CHECK_hvm_altp2m_op and CHECK_hvm_altp2m_set_mem_access_multi. * Removed BUILD_BUG_ON check. * Added comment describing the reason for manually defining the CHECK_ macros. * Added ASSERT_UNREACHABLE as the default switch label action in compat_altp2m_op. * Added ASSERT(rc == __HYPERVISOR_hvm_op) to make sure the return code was actually sey by hypercall_create_continuation. Changed since v7: * Changed the patch title. --- tools/libxc/include/xenctrl.h | 3 + tools/libxc/xc_altp2m.c | 40 xen/arch/x86/hvm/hvm.c | 138 +++- xen/include/Makefile| 1 + xen/include/public/hvm/hvm_op.h | 36 +-- xen/include/xlat.lst| 1 + 6 files changed, 213 insertions(+), 6 deletions(-) diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h index 666db0b..f171668 100644 --- a/tools/libxc/include/xenctrl.h +++ b/tools/libxc/include/xenctrl.h @@ -1974,6 +1974,9 @@ int xc_altp2m_set_mem_access(xc_interface *handle, uint32_t domid, int xc_altp2m_change_gfn(xc_interface *handle, uint32_t domid, uint16_t view_id, xen_pfn_t old_gfn, xen_pfn_t new_gfn); +int xc_altp2m_set_mem_access_multi(xc_interface *handle, uint32_t domid, + uint16_t view_id, uint8_t *access, + uint64_t *pages, uint32_t nr); /** * Mem paging operations. diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c index 07fcd18..e170fe3 100644 --- a/tools/libxc/xc_altp2m.c +++ b/tools/libxc/xc_altp2m.c @@ -213,3 +213,43 @@ int xc_altp2m_change_gfn(xc_interface *handle, uint32_t domid, return rc; } +int xc_altp2m_set_mem_access_multi(xc_interface *xch, uint32_t domid, + uint16_t view_id, uint8_t *access, + uint64_t *pages, uint32_t nr) +{ +int rc; + +DECLARE_HYPERCALL_BUFFER(xen_hvm_altp2m_op_t, arg); +DECLARE_HYPERCALL_BOUNCE(access, nr, XC_HYPERCALL_BUFFER_BOUNCE_IN); +DECLARE_HYPERCALL_BOUNCE(pages, nr * sizeof(uint64_t), + XC_HYPERCALL_BUFFER_BOUNCE_IN); + +arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg)); +if ( arg == NULL ) +return -1; + +arg->version = HVMOP_ALTP2M_INTERFACE_VERSION; +arg->cmd = HVMOP_altp2m_set_mem_access_multi; +arg->domain = domid; +arg->u.set_mem_access_multi.view = view_id; +
Re: [Xen-devel] [PATCH v2] scripts: introduce a script for build test
Wei Liu writes ("[PATCH v2] scripts: introduce a script for build test"): ... > +if git branch | grep -q '^\*.\+detached at'; then You mean some rune involving git-symbolic-ref. git-symbolic-ref -q HEAD exits with status 1 if HEAD is detached, 0 if HEAD is a branch, or some other status in case of trouble. But you could combine this test with your ORIG_BRANCH thing, and just say git-symbolic-ref HEAD which exits 128 if HEAD is detached. > +if ! git merge-base --is-ancestor $BASE $TIP; then > +echo "$BASE is not an ancestor of $TIP, aborted" > +exit 1 > +fi I would remove this check. There is nothing wrong with asking "does this branch build everywhere" even if it hasn't rebased yet. The rest LGTM. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] libxc: remove stale error check for domain size in xc_sr_save_x86_hvm.c
On 23/10/17 11:20, Juergen Gross wrote: > On 06/10/17 15:30, Julien Grall wrote: >> Hi, >> >> On 27/09/17 15:36, Wei Liu wrote: >>> On Tue, Sep 26, 2017 at 02:02:56PM +0200, Juergen Gross wrote: Long ago domains to be saved were limited to 1TB size due to the migration stream v1 limitations which used a 32 bit value for the PFN and the frame type (4 bits) leaving only 28 bits for the PFN. Migration stream V2 uses a 64 bit value for this purpose, so there is no need to refuse saving (or migrating) domains larger than 1 TB. For 32 bit toolstacks there is still a size limit, as domains larger than about 1TB will lead to an exhausted virtual address space of the saving process. So keep the test for 32 bit, but don't base it on the page type macros. As a migration could lead to the situation where a 32 bit toolstack would have to handle such a large domain (in case the sending side is 64 bit) the same test should be added for restoring a domain. Signed-off-by: Juergen Gross>>> I will leave this to Andrew. >>> >>> I don't really have an opinion here. >> >> I will wait Andrew feedback before giving a release ack on this patch. > Andrew? Sorry - this completely fell off my radar. This is probably fine overall. One area which is now more important for vendors to take care over is preventing migration of VMs from swapping dom0 to death. There are a number of large structures allocated (only O(n) with the size of the VM), and this will get worse as/when steps are taken to address the ballooning issues. The best way is probably to limit the number of concurrent migrations which can be performed. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [xen-unstable test] 115037: regressions - FAIL
Hi, On 23/10/2017 16:57, Wei Liu wrote: On Mon, Oct 23, 2017 at 03:38:33PM +0100, Andrew Cooper wrote: On 23/10/17 15:34, Jan Beulich wrote: On 23.10.17 at 15:58,wrote: On 23/10/17 09:40, Jan Beulich wrote: On 23.10.17 at 01:49, wrote: flight 115037 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/115037/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stopfail REGR. vs. 114644 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail REGR. vs. 114644 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail REGR. vs. 114644 I'm puzzled by these recurring failures: Until flight 114525 all three (plus the fourth sibling, which is in "guest-stop fail never pass" state) were fail-never-pass on windows-install (the 64-bit host ones) or guest-saverestore (the 32-bit host ones). Then flights 114540 and 114644 were successes, and since then guest-stop has been failing. The guest console doesn't show any indication that the guest may have received a shutdown signal. Would it be possible of a platform specific bug? The last two flights are failing on merlot1. Not very likely here, I would say. These tests have reliably never passed before, and there are no changes recently (I'm aware of) which would cause them to start passing. There is on osstest side -- we bumped the disk from 10G to 20G. Previously the tests failed due to there was insufficient space to store this iso and the guest image. They seemed to have passed twice on two separate machine and then failed reliably again on merlot1/pinot0. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v9.1 02/16] Rename PSR sysctl/domctl interfaces and xsm policy to make them be general
This patch renames PSR sysctl/domctl interfaces and related xsm policy to make them be general for all resource allocation features but not only for CAT. Then, we can resuse the interfaces for all allocation features. Basically, it changes 'psr_cat_op' to 'psr_alloc', and remove 'CAT_' from some macros. E.g.: 1. psr_cat_op -> psr_alloc 2. XEN_DOMCTL_psr_cat_op -> XEN_DOMCTL_psr_alloc 3. XEN_SYSCTL_psr_cat_op -> XEN_SYSCTL_psr_alloc 4. XEN_DOMCTL_PSR_CAT_SET_L3_CBM -> XEN_DOMCTL_PSR_SET_L3_CBM 5. XEN_SYSCTL_PSR_CAT_get_l3_info -> XEN_SYSCTL_PSR_get_l3_info Signed-off-by: Yi SunReviewed-by: Wei Liu Reviewed-by: Roger Pau Monné Acked-by: Jan Beulich Acked-by: Daniel De Graaf --- CC: Jan Beulich CC: Andrew Cooper CC: Wei Liu CC: Ian Jackson CC: Daniel De Graaf CC: Roger Pau Monné CC: Chao Peng v9: - rename 'psr_cat_op' to 'psr_alloc' in xen.if. v7: - add single trailing underscore for internal variabled in macro. (suggested by Jan Beulich) - add parentheses for input parameters of marcro. (suggested by Jan Beulich) - adjust the postion of macro. (suggested by Jan Beulich) v6: - move macro definition into the function and undefine it after use. (suggested by Roger Pau Monné) - do not bump sysctl version because it has been bumped for 4.10. (suggested by Roger Pau Monné) v5: - remove domctl version number upgrade. (suggested by Jan Beulich) - restore 'XEN_SYSCTL_PSR_CAT_L3_CDP'. (suggested by Jan Beulich) - define a local macro to complete psr get value flow. (suggested by Roger Pau Monné) - remove 'Reviewed-by' and 'Acked-by'. (suggested by Wei Liu) v4: - remove 'ALLOC_' from names. (suggested by Roger Pau Monné) - fix comments. (suggested by Roger Pau Monné) v3: - remove 'op/OP' from names and modify some names from 'PSR_CAT' to 'PSR_ALLOC'. (suggested by Roger Pau Monné) v1: - add description about what to be changed in commit message. (suggested by Wei Liu) - bump sysctl/domctl version numbers. (suggested by Wei Liu) --- tools/flask/policy/modules/dom0.te | 4 +-- tools/flask/policy/modules/xen.if | 2 +- tools/libxc/xc_psr.c| 50 +- xen/arch/x86/domctl.c | 71 ++--- xen/arch/x86/sysctl.c | 28 +++ xen/include/public/domctl.h | 24 ++--- xen/include/public/sysctl.h | 12 +++ xen/xsm/flask/hooks.c | 8 ++--- xen/xsm/flask/policy/access_vectors | 8 ++--- 9 files changed, 103 insertions(+), 104 deletions(-) diff --git a/tools/flask/policy/modules/dom0.te b/tools/flask/policy/modules/dom0.te index 1643b40..07de3d5 100644 --- a/tools/flask/policy/modules/dom0.te +++ b/tools/flask/policy/modules/dom0.te @@ -14,7 +14,7 @@ allow dom0_t xen_t:xen { tmem_control getscheduler setscheduler }; allow dom0_t xen_t:xen2 { - resource_op psr_cmt_op psr_cat_op pmu_ctrl get_symbol + resource_op psr_cmt_op psr_alloc pmu_ctrl get_symbol get_cpu_levelling_caps get_cpu_featureset livepatch_op gcov_op set_parameter }; @@ -39,7 +39,7 @@ allow dom0_t dom0_t:domain { }; allow dom0_t dom0_t:domain2 { set_cpuid gettsc settsc setscheduler set_max_evtchn set_vnumainfo - get_vnumainfo psr_cmt_op psr_cat_op set_gnttab_limits + get_vnumainfo psr_cmt_op psr_alloc set_gnttab_limits }; allow dom0_t dom0_t:resource { add remove }; diff --git a/tools/flask/policy/modules/xen.if b/tools/flask/policy/modules/xen.if index 5543749..cb48a6c 100644 --- a/tools/flask/policy/modules/xen.if +++ b/tools/flask/policy/modules/xen.if @@ -52,7 +52,7 @@ define(`create_domain_common', ` settime setdomainhandle getvcpucontext set_misc_info }; allow $1 $2:domain2 { set_cpuid settsc setscheduler setclaim set_max_evtchn set_vnumainfo get_vnumainfo cacheflush - psr_cmt_op psr_cat_op soft_reset set_gnttab_limits }; + psr_cmt_op psr_alloc soft_reset set_gnttab_limits }; allow $1 $2:security check_context; allow $1 $2:shadow enable; allow $1 $2:mmu { map_read map_write adjust memorymap physmap pinpage mmuext_op updatemp }; diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c index edec4d1..78deba0 100644 --- a/tools/libxc/xc_psr.c +++ b/tools/libxc/xc_psr.c @@ -258,27 +258,27 @@ int xc_psr_cat_set_domain_data(xc_interface *xch, uint32_t domid, switch ( type ) { case XC_PSR_CAT_L3_CBM: -cmd = XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM; +cmd =
[Xen-devel] [distros-debian-snapshot test] 72348: tolerable trouble: blocked/broken/fail/pass
flight 72348 distros-debian-snapshot real [real] http://osstest.xs.citrite.net/~osstest/testlogs/logs/72348/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-arm64-arm64-armhf-daily-netboot-pygrub 1 build-check(1) blocked n/a build-arm64-pvops 2 hosts-allocate broken like 72223 build-arm64 2 hosts-allocate broken like 72223 build-arm64-pvops 3 capture-logs broken like 72223 build-arm64 3 capture-logs broken like 72223 test-amd64-i386-i386-daily-netboot-pvgrub 10 debian-di-install fail like 72223 test-amd64-i386-amd64-daily-netboot-pygrub 10 debian-di-install fail like 72223 test-amd64-amd64-i386-daily-netboot-pygrub 10 debian-di-install fail like 72223 test-armhf-armhf-armhf-daily-netboot-pygrub 10 debian-di-install fail like 72223 test-amd64-amd64-amd64-daily-netboot-pvgrub 10 debian-di-install fail like 72223 test-amd64-amd64-amd64-current-netinst-pygrub 10 debian-di-install fail like 72223 test-amd64-i386-amd64-weekly-netinst-pygrub 10 debian-di-install fail like 72223 test-amd64-amd64-i386-weekly-netinst-pygrub 10 debian-di-install fail like 72223 test-amd64-amd64-amd64-weekly-netinst-pygrub 10 debian-di-install fail like 72223 test-amd64-i386-i386-weekly-netinst-pygrub 10 debian-di-install fail like 72223 test-amd64-i386-amd64-current-netinst-pygrub 10 debian-di-install fail like 72223 test-amd64-i386-i386-current-netinst-pygrub 10 debian-di-install fail like 72223 test-amd64-amd64-i386-current-netinst-pygrub 10 debian-di-install fail like 72223 baseline version: flight 72223 jobs: build-amd64 pass build-arm64 broken build-armhf pass build-i386 pass build-amd64-pvopspass build-arm64-pvopsbroken build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-amd64-daily-netboot-pvgrub fail test-amd64-i386-i386-daily-netboot-pvgrubfail test-amd64-i386-amd64-daily-netboot-pygrub fail test-arm64-arm64-armhf-daily-netboot-pygrub blocked test-armhf-armhf-armhf-daily-netboot-pygrub fail test-amd64-amd64-i386-daily-netboot-pygrub fail test-amd64-amd64-amd64-current-netinst-pygrubfail test-amd64-i386-amd64-current-netinst-pygrub fail test-amd64-amd64-i386-current-netinst-pygrub fail test-amd64-i386-i386-current-netinst-pygrub fail test-amd64-amd64-amd64-weekly-netinst-pygrub fail test-amd64-i386-amd64-weekly-netinst-pygrub fail test-amd64-amd64-i386-weekly-netinst-pygrub fail test-amd64-i386-i386-weekly-netinst-pygrub fail sg-report-flight on osstest.xs.citrite.net logs: /home/osstest/logs images: /home/osstest/images Logs, config files, etc. are available at http://osstest.xs.citrite.net/~osstest/testlogs/logs Test harness code can be found at http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary Push not applicable. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [linux-4.9 test] 115167: regressions - FAIL
flight 115167 linux-4.9 real [real] http://logs.test-lab.xenproject.org/osstest/logs/115167/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail REGR. vs. 114814 Tests which are failing intermittently (not blocking): test-armhf-armhf-xl-arndale 6 xen-install fail in 115140 pass in 115167 test-armhf-armhf-libvirt 16 guest-start/debian.repeat fail in 115140 pass in 115167 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail pass in 115140 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail in 115140 like 114814 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 114814 test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qcow2 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-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass version targeted for testing: linux4d4a6a3f8a12602ce8dc800123715fe7b5c1c3a1 baseline version: linux5d7a76acad403638f635c918cc63d1d44ffa4065 Last test of basis 114814 2017-10-20 20:51:56 Z3 days Testing same since 114845 2017-10-21 16:14:17 Z2 days6 attempts People who touched revisions under test: Alex DeucherAlexandre Belloni Andrew Morton Anoob Soman Arnd Bergmann Bart Van Assche Ben Skeggs Bin Liu Borislav Petkov Christoph Lameter Christophe JAILLET Coly Li Dan Carpenter David Rientjes
[Xen-devel] [PATCH] xen: fix booting ballooned down hvm guest
Commit 96edd61dcf44362d3ef0bed1a5361e0ac7886a63 ("xen/balloon: don't online new memory initially") introduced a regression when booting a HVM domain with memory less than mem-max: instead of ballooning down immediately the system would try to use the memory up to mem-max resulting in Xen crashing the domain. For HVM domains the current size will be reflected in Xenstore node memory/static-max instead of memory/target. Additionally we have to trigger the ballooning process at once. Cc:# 4.13 Fixes: 96edd61dcf44362d3ef0bed1a5361e0ac7886a63 ("xen/balloon: don't online new memory initially") Suggested-by: Boris Ostrovsky Signed-off-by: Juergen Gross --- drivers/xen/xen-balloon.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/xen/xen-balloon.c b/drivers/xen/xen-balloon.c index e89136ab851e..3745748d9644 100644 --- a/drivers/xen/xen-balloon.c +++ b/drivers/xen/xen-balloon.c @@ -57,7 +57,7 @@ static int register_balloon(struct device *dev); static void watch_target(struct xenbus_watch *watch, const char *path, const char *token) { - unsigned long long new_target; + unsigned long long new_target, static_max; int err; static bool watch_fired; static long target_diff; @@ -72,13 +72,19 @@ static void watch_target(struct xenbus_watch *watch, * pages. PAGE_SHIFT converts bytes to pages, hence PAGE_SHIFT - 10. */ new_target >>= PAGE_SHIFT - 10; - if (watch_fired) { - balloon_set_new_target(new_target - target_diff); - return; + + if (!watch_fired) { + watch_fired = true; + err = xenbus_scanf(XBT_NIL, "memory", "static-max", "%llu", + _max); + if (err != 1) + static_max = new_target; + static_max >>= PAGE_SHIFT - 10; + target_diff = xen_pv_domain() ? 0 + : static_max - balloon_stats.target_pages; } - watch_fired = true; - target_diff = new_target - balloon_stats.target_pages; + balloon_set_new_target(new_target - target_diff); } static struct xenbus_watch target_watch = { .node = "memory/target", -- 2.12.3 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3] xen/balloon: don't online new memory initially
On 03/10/17 23:33, Boris Ostrovsky wrote: > On 10/02/2017 05:37 PM, HW42 wrote: >> Juergen Gross: >>> When setting up the Xenstore watch for the memory target size the new >>> watch will fire at once. Don't try to reach the configured target size >>> by onlining new memory in this case, as the current memory size will >>> be smaller in almost all cases due to e.g. BIOS reserved pages. >>> >>> Onlining new memory will lead to more problems e.g. undesired conflicts >>> with NVMe devices meant to be operated as block devices. >>> >>> Instead remember the difference between target size and current size >>> when the watch fires for the first time and apply it to any further >>> size changes, too. >>> >>> In order to avoid races between balloon.c and xen-balloon.c init calls >>> do the xen-balloon.c initialization from balloon.c. >>> >>> Signed-off-by: Juergen Gross>> This patch seems to introduce a regression. If I boot a HVM or PVH >> domain with memory != maxmem then the kernel inside the domain reports >> that it has maxmem available even though Xen reports only what is set as >> memory. Sooner or later Xen logs "out of PoD memory!" and kills the >> domain. If I revert the corresponding commit (96edd61d) then everything >> works as expected. >> >> Tested this with Xen 4.9.0 and Linux 4.13.4. >> > > > Yes, this indeed doesn't look like it's doing the right thing (although > I haven't seen the "out of memory" error). You need to use enough memory (e.g. via memhog). > I wonder whether target_diff should be computed against xenstore's > "static-max" and not "target" and the memory should be ballooned down > immediately and not on a subsequent watch firing. Right. And we need to keep target_diff = 0 for PV domains. Patch coming soon. Juergen ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/4] libxl: use libxl__device_kind string to access device
On Thu, Oct 5, 2017 at 12:30 PM, Oleksandr Grytsovwrote: > From: Oleksandr Grytsov > > In current implementation the path of device XS entry is created with > string from libxl__device_kind enum. But access to the device entry > usually done with hardcoded path. This is source of potential errors. > This patchset changes hardcoded device name in the XS path to string > representation of libxl__device_kind enum. Also it changes "type" field > in libxl__..._devtype structure to keep libxl__device_kind. It allows > to move some duplicated functions to macros. > > Oleksandr Grytsov (4): > libxl: use libxl__device_kind to get device XS entry > libxl: use libxl__device_kind in LIBXL_DEFINE_UPDATE_DEVID > libxl: move libxl__device_from_ to LIBXL_DEFINE_DEVICE_FROM_TYPE > libxl: move ibxl_devid_to_device_... to LIBXL_DEFINE_DEVID_TO_DEVICE > > tools/libxl/libxl_9pfs.c | 21 +++- > tools/libxl/libxl_colo_nic.c | 6 ++-- > tools/libxl/libxl_console.c | 36 +--- > tools/libxl/libxl_create.c| 4 +-- > tools/libxl/libxl_device.c| 10 +++--- > tools/libxl/libxl_disk.c | 28 +++- > tools/libxl/libxl_domain.c| 2 +- > tools/libxl/libxl_internal.h | 77 ++ > ++--- > tools/libxl/libxl_netbuffer.c | 6 ++-- > tools/libxl/libxl_nic.c | 55 +++ > tools/libxl/libxl_pci.c | 21 > tools/libxl/libxl_usb.c | 52 +++-- > tools/libxl/libxl_vdispl.c| 62 ++ > tools/libxl/libxl_vkb.c | 61 +- > tools/libxl/libxl_vsnd.c | 62 ++ > tools/libxl/libxl_vtpm.c | 33 +++ > 16 files changed, 222 insertions(+), 314 deletions(-) > > -- > 2.7.4 > > ping -- Best Regards, Oleksandr Grytsov. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/6] libxl: create standalone vkb device
On Thu, Oct 5, 2017 at 12:07 PM, Oleksandr Grytsovwrote: > From: Oleksandr Grytsov > > Currently vkb device is the part of FB and console. > In embedded application we use vkb protocol to communicate > with user space backend. For this purpose we need to have > possibility to enable vkb device without QEMU, FB etc. > > This particular issue was already discussed int the mail > thread [1]. There were few possible solutions. We've implemented > one suggested by Stefano: add "type" field for vkb. > Each backend (QEMU or user space) shall read this field and > serve frontend only for own type. I will provide the patch > for QEMU backend, once this solution is submitted to libxl. > > This patchset consist of following changes: > > * vkb related code is moved to libxl_vkb.c - as it now > used not only by console and FB; > * add backend type support in order to support QEMU and > user space backends; > * add getting vkb list and getting device by id in order > to implement CLI commands to attach, detach and list > vkb devices; > * add new vkb entry in xl.cfg to handle separate vkb > configuration; > * add CLI vkb-attach, vkb-detach and vkb-list commands; > * update documentation accordingly. > > [1] https://marc.info/?l=qemu-devel=149219237030212=2 > > Oleksandr Grytsov (6): > libxl: move vkb device to libxl_vkb.c > libxl: fix vkb XS entry and type > libxl: add backend type to vkb > libxl: vkb add list and info functions > xl: add vkb config parser and CLI > docs: add vkb device to xl.cfg and xl > > docs/man/xl.cfg.pod.5.in| 24 ++ > docs/man/xl.pod.1.in| 22 ++ > tools/libxl/Makefile| 1 + > tools/libxl/libxl.h | 10 +++ > tools/libxl/libxl_console.c | 53 - > tools/libxl/libxl_create.c | 4 + > tools/libxl/libxl_dm.c | 2 + > tools/libxl/libxl_types.idl | 18 +s > tools/libxl/libxl_utils.h | 3 + > tools/libxl/libxl_vkb.c | 180 ++ > ++ > tools/xl/Makefile | 2 +- > tools/xl/xl.h | 3 + > tools/xl/xl_cmdtable.c | 15 > tools/xl/xl_parse.c | 77 ++- > tools/xl/xl_parse.h | 2 +- > tools/xl/xl_vkb.c | 141 ++ > 16 files changed, 501 insertions(+), 56 deletions(-) > create mode 100644 tools/libxl/libxl_vkb.c > create mode 100644 tools/xl/xl_vkb.c > > -- > 2.7.4 > > ping -- Best Regards, Oleksandr Grytsov. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] Is that possible to merge MBA into Xen 4.10?
>>> On 24.10.17 at 04:10,wrote: > As you may know, MBA patch set has got enough Reviewed-by/Acked-by in last > week. > It is ready to be merged. > > This is a feature for Skylake, Intel has launched Skylake and KVM already > supported MBA, so including it in Xen 4.10 will quickly fill this gap. > > MBA missed the 4.10 feature freeze date for only a few days due to lack of > timely review for earlier versions which slowed down the patch iteration > notably. > It seems maintainers are very busy recently so that the review progress for > 4.10 > is slower than before. So I am wondering if it is possible to merge it into > 4.10? > > This patch set mainly touches codes related to PSR in > tools/domctl/sysctl/hypervisor. > It does not touch other features. So, the risk is low to merge it. While I agree the risk is low, I think we should not start making exceptions from the "no freeze exceptions" rule. Even less so for a secondary (or should I pull out my favorite "niche" again?) feature like this one. But in the end it's Julien's call., of course. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/5] libxl: add PV sound device
On Mon, Oct 2, 2017 at 12:49 PM, Oleksandr Grytsovwrote: > From: Oleksandr Grytsov > > This patch set adds PV sound device support to xl.cfg and xl. > See sndif.h for protocol implementation details. > > > Oleksandr Grytsov (5): > libxl: add PV sound device > libxl: add vsnd list and info > xl: add PV sound condif parser > xl: add vsnd CLI commands > docs: add PV sound device config > > docs/man/xl.cfg.pod.5.in | 150 > docs/man/xl.pod.1.in | 30 ++ > tools/libxl/Makefile | 2 +- > tools/libxl/libxl.h | 24 ++ > tools/libxl/libxl_create.c | 1 + > tools/libxl/libxl_internal.h | 1 + > tools/libxl/libxl_types.idl | 83 + > tools/libxl/libxl_types_internal.idl | 1 + > tools/libxl/libxl_utils.h| 3 + > tools/libxl/libxl_vsnd.c | 660 ++ > + > tools/xl/Makefile| 2 +- > tools/xl/xl.h| 3 + > tools/xl/xl_cmdtable.c | 15 + > tools/xl/xl_parse.c | 250 + > tools/xl/xl_parse.h | 1 + > tools/xl/xl_vsnd.c | 203 +++ > 16 files changed, 1427 insertions(+), 2 deletions(-) > create mode 100644 tools/libxl/libxl_vsnd.c > create mode 100644 tools/xl/xl_vsnd.c > > -- > 2.7.4 > > ping -- Best Regards, Oleksandr Grytsov. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V3 1/29] Xen/doc: Add Xen virtual IOMMU doc
On 2017年10月19日 19:28, Jan Beulich wrote: On 19.10.17 at 10:49,wrote: >> On Thu, Oct 19, 2017 at 10:26:36AM +0800, Lan Tianyu wrote: >>> Hi Roger: >>> Thanks for review. >>> >>> On 2017年10月18日 21:26, Roger Pau Monné wrote: On Thu, Sep 21, 2017 at 11:01:42PM -0400, Lan Tianyu wrote: > +Xen hypervisor vIOMMU command > += > +Introduce vIOMMU command "viommu=1" to enable vIOMMU function in >> hypervisor. > +It's default disabled. Hm, I'm not sure we really need this. At the end viommu will be disabled by default for guests, unless explicitly enabled in the config file. >>> >>> This is according to Jan's early comments on RFC patch >>> https://patchwork.kernel.org/patch/9733869/. >>> >>> "It's actually a question whether in our current scheme a Kconfig >>> option is appropriate here in the first place. I'd rather see this be >>> an always built feature which needs enabling on the command line >>> for the time being." >> >> So if I read this correctly Jan wanted you to ditch the Kconfig option >> and instead rely on the command line option to enable/disable it. > > Yes. > > Jan > OK. I will remove the command in the next version. Thanks for clarification. -- Best regards Tianyu Lan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [qemu-mainline test] 115169: regressions - FAIL
flight 115169 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/115169/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-i386-xsm6 xen-buildfail REGR. vs. 114507 build-i3866 xen-buildfail REGR. vs. 114507 build-amd64-xsm 6 xen-buildfail REGR. vs. 114507 build-amd64 6 xen-buildfail REGR. vs. 114507 build-armhf-xsm 6 xen-buildfail REGR. vs. 114507 build-armhf 6 xen-buildfail REGR. vs. 114507 Tests which did not succeed, but are not blocking: test-amd64-i386-libvirt-qcow2 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64 1 build-check(1)blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-intel 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win10-i386 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-amd64-pair 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit2 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win10-i386 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-amd64-pygrub 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qcow2 1 build-check(1) blocked n/a test-amd64-amd64-amd64-pvgrub 1 build-check(1) blocked n/a test-armhf-armhf-xl-arndale 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-xl-credit2 1 build-check(1) blocked n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-i386-xl-xsm1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 1 build-check(1)blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-raw1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-amd64-i386-pvgrub 1 build-check(1) blocked n/a build-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvhv2-intel 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-xsm 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-intel 1 build-check(1) blocked n/a test-armhf-armhf-xl-vhd 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl 1 build-check(1) blocked n/a test-amd64-i386-pair 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvhv2-amd 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-amd 1 build-check(1) blocked n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) blocked n/a test-armhf-armhf-xl-rtds 1
[Xen-devel] [xen-unstable test] 115163: regressions - FAIL
flight 115163 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/115163/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stopfail REGR. vs. 114644 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail REGR. vs. 114644 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail REGR. vs. 114644 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-qemuu-ovmf-amd64 14 guest-localmigrate fail REGR. vs. 114644 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail like 114644 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 114644 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 114644 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 114644 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 114644 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 114644 test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qcow2 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass version targeted for testing: xen 1f2c7894dfe3d52f33655de202bd474999a1637b baseline version: xen 24fb44e971a62b345c7b6ca3c03b454a1e150abe Last test of basis 114644 2017-10-17 10:49:11 Z6 days Failing since114670 2017-10-18 05:03:38 Z6 days9 attempts Testing same since 115163 2017-10-23 21:47:10 Z0 days1 attempts People who touched revisions under test: Andrew CooperAnthony PERARD David Esler George Dunlap Ian Jackson Jan Beulich Julien Grall Roger Pau Monné Stefano Stabellini Tim Deegan Wei Liu jobs: build-amd64-xsm pass build-armhf-xsm