[qemu-mainline test] 151459: regressions - FAIL
flight 151459 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/151459/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-amd64-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-amd64-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-amd64-xl-qemuu-win7-amd64 10 windows-install fail REGR. vs. 151065 test-amd64-amd64-libvirt-vhd 10 debian-di-installfail REGR. vs. 151065 test-amd64-i386-xl-qemuu-win7-amd64 10 windows-install fail REGR. vs. 151065 test-amd64-amd64-qemuu-nested-intel 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-libvirt-xsm 12 guest-start fail REGR. vs. 151065 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-amd64-qemuu-nested-amd 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-amd64-xl-qcow210 debian-di-installfail REGR. vs. 151065 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-qemuu-rhel6hvm-amd 10 redhat-install fail REGR. vs. 151065 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-xl-qemuu-ws16-amd64 10 windows-install fail REGR. vs. 151065 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-libvirt-pair 21 guest-start/debian fail REGR. vs. 151065 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-freebsd10-i386 11 guest-startfail REGR. vs. 151065 test-amd64-i386-qemuu-rhel6hvm-intel 10 redhat-install fail REGR. vs. 151065 test-amd64-amd64-libvirt-xsm 12 guest-start fail REGR. vs. 151065 test-amd64-amd64-libvirt 12 guest-start fail REGR. vs. 151065 test-amd64-i386-freebsd10-amd64 11 guest-start fail REGR. vs. 151065 test-amd64-i386-libvirt 12 guest-start fail REGR. vs. 151065 test-amd64-amd64-libvirt-pair 21 guest-start/debian fail REGR. vs. 151065 test-armhf-armhf-xl-vhd 10 debian-di-installfail REGR. vs. 151065 test-arm64-arm64-libvirt-xsm 12 guest-start fail REGR. vs. 151065 test-armhf-armhf-libvirt-raw 10 debian-di-installfail REGR. vs. 151065 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-install fail REGR. vs. 151065 test-armhf-armhf-libvirt 12 guest-start fail REGR. vs. 151065 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-pvshim12 guest-start fail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13
[linux-linus test] 151452: regressions - FAIL
flight 151452 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/151452/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-arm64-arm64-libvirt-xsm 16 guest-start/debian.repeat fail REGR. vs. 151214 Tests which are failing intermittently (not blocking): test-amd64-i386-xl-pvshim 7 xen-boot fail pass in 151441 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-pvshim12 guest-start fail in 151441 never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 151214 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 151214 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 151214 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 151214 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 151214 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 151214 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 151214 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 151214 test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-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-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass version targeted for testing: linux9ebcfadb0610322ac537dd7aa5d9cbc2b2894c68 baseline version: linux1b5044021070efa3259f3e9548dc35d1eb6aa844 Last test of basis 151214 2020-06-18 02:27:46 Z 11 days Failing since151236 2020-06-19 19:10:35 Z 10 days 12 attempts Testing same since 151441 2020-06-28 23:39:48 Z1 days2 attempts
Re: [RFC XEN PATCH 19/23] riscv: Add the lib directory
On Mon, Jun 22, 2020 at 01:38:20PM +0200, Jan Beulich wrote: > On 22.01.2020 02:58, Bobby Eshleman wrote: > > From: Alistair Francis > > > > Add the lib directory with find_next_bit support. > > > > This was taken from Linux > > As was Arm64's - the two definitely would want folding. > Indeed. I'm finding a good more overlap with arch/arm as I move forward, so expect to see a good degree of folding between the two in v2.
Re: [RFC XEN PATCH 22/23] riscv: Add sysctl.c
On Mon, Jun 22, 2020 at 01:43:40PM +0200, Jan Beulich wrote:
> On 22.01.2020 02:59, Bobby Eshleman wrote:
> > --- /dev/null
> > +++ b/xen/arch/riscv/sysctl.c
> > @@ -0,0 +1,31 @@
> > +/**
> > + * Arch-specific sysctl.c
> > + *
> > + * System management operations. For use by node control stack.
> > + *
> > + * Copyright (c) 2012, Citrix Systems
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +void arch_do_physinfo(struct xen_sysctl_physinfo *pi) { }
> > +
> > +long arch_do_sysctl(struct xen_sysctl *sysctl,
> > +XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
> > +{
> > +return -ENOSYS;
>
> At the example of this (there may be more in this series) - -EOPNOTSUPP
> please. Only top level hypercall handlers ought to produce -ENOSYS, for
> major hypercall numbers with no handler.
>
Got it, will do. Thanks!
Re: [RFC XEN PATCH 00/23] xen: beginning support for RISC-V
On Tue, Jun 16, 2020 at 01:16:10PM -0700, Stefano Stabellini wrote: > On Mon, 15 Jun 2020, Bobby Eshleman wrote: > > On Tue, Jun 16, 2020 at 01:10:17AM +, Alistair Francis wrote: > > > On Mon, 2020-06-15 at 18:03 -0700, Stefano Stabellini wrote: > > > > Any updates? I am looking forward to this :-) > > > > > > > It has been on a slow burn since I became a new dad (shortly after the RFC). > > I've gradually regained free time, and so I've been able to change that > > slow burn to a moderate burn in the last couple weeks. > > > > Most of my progress has been around build environment improvements. I've > > done > > some work stripping it down to the bare minimum required to build a new arch > > and rooting the commit history from there, and some work with incorporating > > it > > into the gitlab CI, containerizing the build and QEMU run, etc... > > > > As far as hypervisor status: I'm just about done with incorporating the > > boot > > module FDT parsing code, extracting kernel info and ram regions > > (taken/generalized from arch/arm), plus implementing the arch-specific > > pieces > > of domain_create(). > > > > On the verge of being able to dive into a guest kernel and see what breaks > > first :) > > > > I'm expected to commit an extra day or two per week in the next month or so > > at > > Vates, so this will considerably bump up my cadence in comparison to the > > last > > few months. > > Great to hear and congratulations! I'll stay tuned. Next time I'll try > to rebuild and run your series on QEMU, I might ask you for some help > with the setup. > Thanks! And absolutely, feel free :)
RE: [PATCH] xen: introduce xen_vring_use_dma
> Subject: RE: [PATCH] xen: introduce xen_vring_use_dma > > On Mon, 29 Jun 2020, Peng Fan wrote: > > > > If that is the case, how is it possible that virtio breaks on ARM > > > > using the default dma_ops? The breakage is not Xen related (except > > > > that Xen turns dma_ops on). The original message from Peng was: > > > > > > > > vring_map_one_sg -> vring_use_dma_api > > > >-> dma_map_page > > > >-> __swiotlb_map_page > > > > ->swiotlb_map_page > > > > > > > > ->__dma_map_area(phys_to_virt(dma_to_phys(dev, > > > dev_addr)), size, dir); > > > > However we are using per device dma area for rpmsg, phys_to_virt > > > > could not return a correct virtual address for virtual address in > > > > vmalloc area. Then kernel panic. > > > > > > > > I must be missing something. Maybe it is because it has to do with > RPMesg? > > > > > > I think it's an RPMesg bug, yes > > > > rpmsg bug is another issue, it should not use dma_alloc_coherent for > > reserved area, and use vmalloc_to_page. > > > > Anyway here using dma api will also trigger issue. > > Is the stack trace above for the RPMesg issue or for the Trusty issue? The stack trace you pasted is rpmsg issue. > If it is the stack trace for RPMesg, can you also post the Trusty stack > trace? Or > are they indentical? There is no stack dump here. It successfully using swiotlb to do a map, but we actually no need swiotlb in domu to do the map. Thanks, Peng.
RE: [PATCH] xen: introduce xen_vring_use_dma
On Mon, 29 Jun 2020, Peng Fan wrote: > > > If that is the case, how is it possible that virtio breaks on ARM > > > using the default dma_ops? The breakage is not Xen related (except > > > that Xen turns dma_ops on). The original message from Peng was: > > > > > > vring_map_one_sg -> vring_use_dma_api > > >-> dma_map_page > > > -> __swiotlb_map_page > > > ->swiotlb_map_page > > > > > > ->__dma_map_area(phys_to_virt(dma_to_phys(dev, > > dev_addr)), size, dir); > > > However we are using per device dma area for rpmsg, phys_to_virt > > > could not return a correct virtual address for virtual address in > > > vmalloc area. Then kernel panic. > > > > > > I must be missing something. Maybe it is because it has to do with RPMesg? > > > > I think it's an RPMesg bug, yes > > rpmsg bug is another issue, it should not use dma_alloc_coherent for reserved > area, > and use vmalloc_to_page. > > Anyway here using dma api will also trigger issue. Is the stack trace above for the RPMesg issue or for the Trusty issue? If it is the stack trace for RPMesg, can you also post the Trusty stack trace? Or are they indentical?
Re: [PATCH] xen: introduce xen_vring_use_dma
On Fri, 26 Jun 2020, Michael S. Tsirkin wrote: > On Thu, Jun 25, 2020 at 10:31:27AM -0700, Stefano Stabellini wrote: > > On Wed, 24 Jun 2020, Michael S. Tsirkin wrote: > > > On Wed, Jun 24, 2020 at 02:53:54PM -0700, Stefano Stabellini wrote: > > > > On Wed, 24 Jun 2020, Michael S. Tsirkin wrote: > > > > > On Wed, Jun 24, 2020 at 10:59:47AM -0700, Stefano Stabellini wrote: > > > > > > On Wed, 24 Jun 2020, Michael S. Tsirkin wrote: > > > > > > > On Wed, Jun 24, 2020 at 05:17:32PM +0800, Peng Fan wrote: > > > > > > > > Export xen_swiotlb for all platforms using xen swiotlb > > > > > > > > > > > > > > > > Use xen_swiotlb to determine when vring should use dma APIs to > > > > > > > > map the > > > > > > > > ring: when xen_swiotlb is enabled the dma API is required. When > > > > > > > > it is > > > > > > > > disabled, it is not required. > > > > > > > > > > > > > > > > Signed-off-by: Peng Fan > > > > > > > > > > > > > > Isn't there some way to use VIRTIO_F_IOMMU_PLATFORM for this? > > > > > > > Xen was there first, but everyone else is using that now. > > > > > > > > > > > > Unfortunately it is complicated and it is not related to > > > > > > VIRTIO_F_IOMMU_PLATFORM :-( > > > > > > > > > > > > > > > > > > The Xen subsystem in Linux uses dma_ops via swiotlb_xen to translate > > > > > > foreign mappings (memory coming from other VMs) to physical > > > > > > addresses. > > > > > > On x86, it also uses dma_ops to translate Linux's idea of a physical > > > > > > address into a real physical address (this is unneeded on ARM.) > > > > > > > > > > > > > > > > > > So regardless of VIRTIO_F_IOMMU_PLATFORM, dma_ops should be used on > > > > > > Xen/x86 > > > > > > always and on Xen/ARM if Linux is Dom0 (because it has foreign > > > > > > mappings.) That is why we have the if (xen_domain) return true; in > > > > > > vring_use_dma_api. > > > > > > > > > > VIRTIO_F_IOMMU_PLATFORM makes guest always use DMA ops. > > > > > > > > > > Xen hack predates VIRTIO_F_IOMMU_PLATFORM so it *also* > > > > > forces DMA ops even if VIRTIO_F_IOMMU_PLATFORM is clear. > > > > > > > > > > Unfortunately as a result Xen never got around to > > > > > properly setting VIRTIO_F_IOMMU_PLATFORM. > > > > > > > > I don't think VIRTIO_F_IOMMU_PLATFORM would be correct for this because > > > > the usage of swiotlb_xen is not a property of virtio, > > > > > > > > > Basically any device without VIRTIO_F_ACCESS_PLATFORM > > > (that is it's name in latest virtio spec, VIRTIO_F_IOMMU_PLATFORM is > > > what linux calls it) is declared as "special, don't follow normal rules > > > for access". > > > > > > So yes swiotlb_xen is not a property of virtio, but what *is* a property > > > of virtio is that it's not special, just a regular device from DMA POV. > > > > I am trying to understand what you meant but I think I am missing > > something. > > > > Are you saying that modern virtio should always have > > VIRTIO_F_ACCESS_PLATFORM, hence use normal dma_ops as any other devices? > > I am saying it's a safe default. Clear VIRTIO_F_ACCESS_PLATFORM if you > have some special needs e.g. you are very sure it's ok to bypass DMA > ops, or you need to support a legacy guest (produced in the window > between virtio 1 support in 2014 and support for > VIRTIO_F_ACCESS_PLATFORM in 2016). Ok thanks. I understand and it makes sense to me. > > > > > > You might have noticed that I missed one possible case above: > > > > > > Xen/ARM > > > > > > DomU :-) > > > > > > > > > > > > Xen/ARM domUs don't need swiotlb_xen, it is not even initialized. > > > > > > So if > > > > > > (xen_domain) return true; would give the wrong answer in that case. > > > > > > Linux would end up calling the "normal" dma_ops, not swiotlb-xen, > > > > > > and > > > > > > the "normal" dma_ops fail. > > > > > > > > > > > > > > > > > > The solution I suggested was to make the check in vring_use_dma_api > > > > > > more > > > > > > flexible by returning true if the swiotlb_xen is supposed to be > > > > > > used, > > > > > > not in general for all Xen domains, because that is what the check > > > > > > was > > > > > > really meant to do. > > > > > > > > > > Why not fix DMA ops so they DTRT (nop) on Xen/ARM DomU? What is wrong > > > > > with that? > > > > > > > > swiotlb-xen is not used on Xen/ARM DomU, the default dma_ops are the > > > > ones that are used. So you are saying, why don't we fix the default > > > > dma_ops to work with virtio? > > > > > > > > It is bad that the default dma_ops crash with virtio, so yes I think it > > > > would be good to fix that. However, even if we fixed that, the if > > > > (xen_domain()) check in vring_use_dma_api is still a problem. > > > > > > Why is it a problem? It just makes virtio use DMA API. > > > If that in turn works, problem solved. > > > > You are correct in the sense that it would work. However I do think it > > is wrong for vring_use_dma_api to enable dma_ops/swiotlb-xen for Xen/ARM > > DomUs that don't need it. There are many
[xen-unstable-smoke test] 151457: tolerable all pass - PUSHED
flight 151457 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/151457/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 0e2e54966af556f4047c1048855c4a071028a32d baseline version: xen da53345dd5ff7d3a34e83587fd375c0b7722f46c Last test of basis 151454 2020-06-29 13:01:35 Z0 days Testing same since 151457 2020-06-29 17:01:22 Z0 days1 attempts People who touched revisions under test: Roger Pau Monné jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git da53345dd5..0e2e54966a 0e2e54966af556f4047c1048855c4a071028a32d -> smoke
[xen-unstable test] 151448: tolerable FAIL
flight 151448 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/151448/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 151437 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 151437 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 151437 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 151437 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 151437 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 151437 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 151437 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 151437 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 151437 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 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-raw 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass version targeted for testing: xen 88cfd062e8318dfeb67c7d2eb50b6cd224b0738a baseline version: xen 88cfd062e8318dfeb67c7d2eb50b6cd224b0738a Last test of basis 151448 2020-06-29 06:27:51 Z0 days Testing same since (not found) 0 attempts jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64-xtf pass build-amd64 pass build-arm64
[qemu-mainline test] 151445: regressions - trouble: broken/fail/pass
flight 151445 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/151445/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl-credit1 broken test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-amd64-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-amd64-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-amd64-xl-qemuu-win7-amd64 10 windows-install fail REGR. vs. 151065 test-amd64-amd64-libvirt-vhd 10 debian-di-installfail REGR. vs. 151065 test-amd64-i386-xl-qemuu-win7-amd64 10 windows-install fail REGR. vs. 151065 test-amd64-amd64-qemuu-nested-intel 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-libvirt-xsm 12 guest-start fail REGR. vs. 151065 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-amd64-qemuu-nested-amd 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-amd64-xl-qcow210 debian-di-installfail REGR. vs. 151065 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-qemuu-rhel6hvm-amd 10 redhat-install fail REGR. vs. 151065 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-xl-qemuu-ws16-amd64 10 windows-install fail REGR. vs. 151065 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-libvirt-pair 21 guest-start/debian fail REGR. vs. 151065 test-amd64-i386-freebsd10-i386 11 guest-startfail REGR. vs. 151065 test-amd64-i386-qemuu-rhel6hvm-intel 10 redhat-install fail REGR. vs. 151065 test-amd64-amd64-libvirt-xsm 12 guest-start fail REGR. vs. 151065 test-amd64-amd64-libvirt 12 guest-start fail REGR. vs. 151065 test-amd64-i386-freebsd10-amd64 11 guest-start fail REGR. vs. 151065 test-amd64-i386-libvirt 12 guest-start fail REGR. vs. 151065 test-amd64-amd64-libvirt-pair 21 guest-start/debian fail REGR. vs. 151065 test-armhf-armhf-xl-vhd 10 debian-di-installfail REGR. vs. 151065 test-arm64-arm64-libvirt-xsm 12 guest-start fail REGR. vs. 151065 test-armhf-armhf-libvirt-raw 10 debian-di-installfail REGR. vs. 151065 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-install fail REGR. vs. 151065 test-armhf-armhf-libvirt 12 guest-start fail REGR. vs. 151065 Tests which are failing intermittently (not blocking): test-armhf-armhf-xl-credit1 4 host-install(4) broken pass in 151435 test-arm64-arm64-xl-seattle 7 xen-boot fail in 151435 pass in 151445 Tests which did not succeed, but are not blocking: test-armhf-armhf-xl-rtds 17 guest-start.2 fail in 151435 blocked in 151065 test-armhf-armhf-xl-credit1 13 migrate-support-check fail in 151435 never pass test-armhf-armhf-xl-credit1 14 saverestore-support-check fail in 151435 never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass
Re: [PATCH] arm/xen: remove the unused macro GRANT_TABLE_PHYSADDR
On Sun, 28 Jun 2020, Xiaofei Tan wrote:
> Fix the following sparse warning:
>
> arch/arm64/xen/../../arm/xen/enlighten.c:244: warning: macro
> "GRANT_TABLE_PHYSADDR" is not used [-Wunused-macros]
>
> It is an isolated macro, and should be removed when its last user
> was deleted in the following commit 3cf4095d7446 ("arm/xen: Use
> xen_xlate_map_ballooned_pages to setup grant table")
>
> Signed-off-by: Xiaofei Tan
Reviewed-by: Stefano Stabellini
> ---
> arch/arm/xen/enlighten.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index fd4e1ce1..e93145d 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -241,7 +241,6 @@ static int __init fdt_find_hyper_node(unsigned long node,
> const char *uname,
> * see Documentation/devicetree/bindings/arm/xen.txt for the
> * documentation of the Xen Device Tree format.
> */
> -#define GRANT_TABLE_PHYSADDR 0
> void __init xen_early_init(void)
> {
> of_scan_flat_dt(fdt_find_hyper_node, NULL);
> --
> 2.7.4
>
Re: [PATCH] build: tweak variable exporting for make 3.82
On Fri, Jun 26, 2020 at 05:02:30PM +0200, Jan Beulich wrote:
> While I've been running into an issue here only because of an additional
> local change I'm carrying, to be able to override just the compiler in
> $(XEN_ROOT)/.config (rather than the whole tool chain), in
> config/StdGNU.mk:
>
> ifeq ($(filter-out default undefined,$(origin CC)),)
>
> I'd like to propose to nevertheless correct the underlying issue:
> Exporting an unset variable changes its origin from "undefined" to
> "file". This comes into effect because of our adding of -rR to
> MAKEFLAGS, which make 3.82 wrongly applies also upon re-invoking itself
> after having updated auto.conf{,.cmd}.
>
> Move the export statement past $(XEN_ROOT)/config/$(XEN_OS).mk inclusion
> such that the variables already have their designated values at that
> point, while retaining their initial origin up to the point they get
> defined.
>
> Signed-off-by: Jan Beulich
>
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -17,8 +17,6 @@ export XEN_BUILD_HOST ?= $(shell hostnam
> PYTHON_INTERPRETER := $(word 1,$(shell which python3 python python2
> 2>/dev/null) python)
> export PYTHON?= $(PYTHON_INTERPRETER)
>
> -export CC CXX LD
> -
> export BASEDIR := $(CURDIR)
> export XEN_ROOT := $(BASEDIR)/..
>
> @@ -42,6 +40,8 @@ export TARGET_ARCH := $(shell echo $
> # Allow someone to change their config file
> export KCONFIG_CONFIG ?= .config
>
> +export CC CXX LD
> +
> .PHONY: default
> default: build
That patch is fine and it is probably better to export a variable that
has a value rather than before the variable is set.
Reviewed-by: Anthony PERARD
Thanks,
--
Anthony PERARD
[PATCH] x86: fix compat header generation
As was pointed out by "mm: fix public declaration of struct
xen_mem_acquire_resource", we're not currently handling structs
correctly that has uint64_aligned_t fields. #pragma pack(4) suppresses
the necessary alignment even if the type did properly survive (which
it also didn't) in the process of generating the headers. Overall,
with the above mentioned change applied, there's only a latent issue
here afaict, i.e. no other of our interface structs is currently
affected.
As a result it is clear that using #pragma pack(4) is not an option.
Drop all uses from compat header generation. Make sure
{,u}int64_aligned_t actually survives, such that explicitly aligned
fields will remain aligned. Arrange for {,u}int64_t to be transformed
into a type that's 64 bits wide and 4-byte aligned, by utilizing that
in typedef-s the "aligned" attribute can be used to reduce alignment.
Note that this changes alignment (but not size) of certain compat
structures, when one or more of their fields use a non-translated struct
as their type(s). This isn't correct, and hence applying alignof() to
such fields requires care, but the prior situation was even worse.
There's one change to generated code according to my observations: In
arch_compat_vcpu_op() the runstate area "area" variable would previously
have been put in a just 4-byte aligned stack slot (despite being 8 bytes
in size), whereas now it gets put in an 8-byte aligned location.
Signed-off-by: Jan Beulich
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -34,15 +34,6 @@ headers-$(CONFIG_XSM_FLASK) += compat/xs
cppflags-y:= -include public/xen-compat.h
-DXEN_GENERATING_COMPAT_HEADERS
cppflags-$(CONFIG_X86)+= -m32
-# 8-byte types are 4-byte aligned on x86_32 ...
-ifeq ($(CONFIG_CC_IS_CLANG),y)
-prefix-$(CONFIG_X86) := \#pragma pack(push, 4)
-suffix-$(CONFIG_X86) := \#pragma pack(pop)
-else
-prefix-$(CONFIG_X86) := \#pragma pack(4)
-suffix-$(CONFIG_X86) := \#pragma pack()
-endif
-
endif
public-$(CONFIG_X86) := $(wildcard public/arch-x86/*.h public/arch-x86/*/*.h)
@@ -57,16 +48,16 @@ compat/%.h: compat/%.i Makefile $(BASEDI
echo "#define $$id" >>$@.new; \
echo "#include " >>$@.new; \
$(if $(filter-out compat/arch-%.h,$@),echo "#include <$(patsubst
compat/%,public/%,$@)>" >>$@.new;) \
- $(if $(prefix-y),echo "$(prefix-y)" >>$@.new;) \
grep -v '^# [0-9]' $< | \
$(PYTHON) $(BASEDIR)/tools/compat-build-header.py | uniq >>$@.new; \
- $(if $(suffix-y),echo "$(suffix-y)" >>$@.new;) \
echo "#endif /* $$id */" >>$@.new
mv -f $@.new $@
+.PRECIOUS: compat/%.i
compat/%.i: compat/%.c Makefile
$(CPP) $(filter-out -Wa$(comma)% -include
%/include/xen/config.h,$(XEN_CFLAGS)) $(cppflags-y) -o $@ $<
+.PRECIOUS: compat/%.c
compat/%.c: public/%.h xlat.lst Makefile
$(BASEDIR)/tools/compat-build-source.py
mkdir -p $(@D)
grep -v 'DEFINE_XEN_GUEST_HANDLE(long)' $< | \
--- a/xen/tools/compat-build-header.py
+++ b/xen/tools/compat-build-header.py
@@ -3,7 +3,7 @@
import re,sys
pats = [
- [ r"__InClUdE__(.*)", r"#include\1\n#pragma pack(4)" ],
+ [ r"__InClUdE__(.*)", r"#include\1" ],
[ r"__IfDeF__ (XEN_HAVE.*)", r"#ifdef \1" ],
[ r"__ElSe__", r"#else" ],
[ r"__EnDif__", r"#endif" ],
@@ -13,7 +13,8 @@ pats = [
[ r"(struct|union|enum)\s+(xen_?)?(\w)", r"\1 compat_\3" ],
[ r"@KeeP@", r"" ],
[ r"_t([^\w]|$)", r"_compat_t\1" ],
- [ r"(8|16|32|64)_compat_t([^\w]|$)", r"\1_t\2" ],
+ [ r"(8|16|32|64_aligned)_compat_t([^\w]|$)", r"\1_t\2" ],
+ [ r"(\su?int64(_compat)?)_T([^\w]|$)", r"\1_t\3" ],
[ r"(^|[^\w])xen_?(\w*)_compat_t([^\w]|$$)", r"\1compat_\2_t\3" ],
[ r"(^|[^\w])XEN_?", r"\1COMPAT_" ],
[ r"(^|[^\w])Xen_?", r"\1Compat_" ],
--- a/xen/tools/compat-build-source.py
+++ b/xen/tools/compat-build-source.py
@@ -9,6 +9,7 @@ pats = [
[ r"^\s*#\s*endif /\* (XEN_HAVE.*) \*/\s+", r"__EnDif__" ],
[ r"^\s*#\s*define\s+([A-Z_]*_GUEST_HANDLE)", r"#define HIDE_\1" ],
[ r"^\s*#\s*define\s+([a-z_]*_guest_handle)", r"#define hide_\1" ],
+ [ r"^\s*#\s*define\s+(u?int64)_aligned_t\s.*", r"typedef \1_T
__attribute__((aligned(4))) \1_compat_T;" ],
[ r"XEN_GUEST_HANDLE(_[0-9A-Fa-f]+)?", r"COMPAT_HANDLE" ],
];
Re: [PATCH v4 for-4.14 2/2] pvcalls: Document correctly and explicitely the padding for all arches
On Sat, 27 Jun 2020, Julien Grall wrote:
> From: Julien Grall
>
> The specification of pvcalls suggests there is padding for 32-bit x86
> at the end of most the structure. However, they are not described in
> in the public header.
>
> Because of that all the structures would be 32-bit aligned and not
> 64-bit aligned for 32-bit x86.
>
> For all the other architectures supported (Arm and 64-bit x86), the
> structure are aligned to 64-bit because they contain uint64_t field.
> Therefore all the structures contain implicit padding.
>
> Given the specification is authoriitative, the padding will the same for
> the all architectures. The potential breakage of compatibility is ought
> to be fine as pvcalls is still a tech preview.
>
> As an aside, the padding sadly cannot be mandated to be 0 as they are
> already present. So it is not going to be possible to use the padding
> for extending a command in the future.
>
> Signed-off-by: Julien Grall
Aside from typos other mentioned:
Reviewed-by: Stefano Stabellini
> ---
> This wants to be included in Xen 4.14 to avoid more users to rely on
> wrong public headers.
>
> Changes in v4:
> - Revert back to v1 for the patch and expand the commit message
>
> Changes in v3:
> - Use __i386__ rather than CONFIG_X86_32
>
> Changes in v2:
> - It is not possible to use the same padding for 32-bit x86 and
> all the other supported architectures.
> ---
> docs/misc/pvcalls.pandoc| 8
> xen/include/public/io/pvcalls.h | 4
> 2 files changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/docs/misc/pvcalls.pandoc b/docs/misc/pvcalls.pandoc
> index 665dad556c39..971cd8f4b122 100644
> --- a/docs/misc/pvcalls.pandoc
> +++ b/docs/misc/pvcalls.pandoc
> @@ -246,9 +246,7 @@ The format is defined as follows:
> uint32_t domain;
> uint32_t type;
> uint32_t protocol;
> - #ifdef CONFIG_X86_32
> uint8_t pad[4];
> - #endif
> } socket;
> struct xen_pvcalls_connect {
> uint64_t id;
> @@ -257,16 +255,12 @@ The format is defined as follows:
> uint32_t flags;
> grant_ref_t ref;
> uint32_t evtchn;
> - #ifdef CONFIG_X86_32
> uint8_t pad[4];
> - #endif
> } connect;
> struct xen_pvcalls_release {
> uint64_t id;
> uint8_t reuse;
> - #ifdef CONFIG_X86_32
> uint8_t pad[7];
> - #endif
> } release;
> struct xen_pvcalls_bind {
> uint64_t id;
> @@ -276,9 +270,7 @@ The format is defined as follows:
> struct xen_pvcalls_listen {
> uint64_t id;
> uint32_t backlog;
> - #ifdef CONFIG_X86_32
> uint8_t pad[4];
> - #endif
> } listen;
> struct xen_pvcalls_accept {
> uint64_t id;
> diff --git a/xen/include/public/io/pvcalls.h b/xen/include/public/io/pvcalls.h
> index 905880735dda..6da6b5533a10 100644
> --- a/xen/include/public/io/pvcalls.h
> +++ b/xen/include/public/io/pvcalls.h
> @@ -68,6 +68,7 @@ struct xen_pvcalls_request {
> uint32_t domain;
> uint32_t type;
> uint32_t protocol;
> +uint8_t pad[4];
> } socket;
> struct xen_pvcalls_connect {
> uint64_t id;
> @@ -76,10 +77,12 @@ struct xen_pvcalls_request {
> uint32_t flags;
> grant_ref_t ref;
> uint32_t evtchn;
> +uint8_t pad[4];
> } connect;
> struct xen_pvcalls_release {
> uint64_t id;
> uint8_t reuse;
> +uint8_t pad[7];
> } release;
> struct xen_pvcalls_bind {
> uint64_t id;
> @@ -89,6 +92,7 @@ struct xen_pvcalls_request {
> struct xen_pvcalls_listen {
> uint64_t id;
> uint32_t backlog;
> +uint8_t pad[4];
> } listen;
> struct xen_pvcalls_accept {
> uint64_t id;
> --
> 2.17.1
>
Re: [PATCH v3 7/7] tools/proctrace: add proctrace tool
On Fri, Jun 26, 2020 at 7:26 AM Ian Jackson wrote:
>
> Wei Liu writes ("Re: [PATCH v3 7/7] tools/proctrace: add proctrace tool"):
> > On Mon, Jun 22, 2020 at 08:12:56PM +0200, Michał Leszczyński wrote:
> > > Add an demonstration tool that uses xc_vmtrace_* calls in order
> > > to manage external IPT monitoring for DomU.
> ...
> > > +if (rc) {
> > > +fprintf(stderr, "Failed to call xc_vmtrace_pt_disable\n");
> > > +return 1;
> > > +}
> > > +
> >
> > You should close fmem and xc in the exit path.
>
> Thanks for reviewing this. I agree with your comments. But I
> disagree with this one.
>
> This is in main(). When the program exits, the xc handle and memory
> mappings will go away as the kernel tears down the process.
>
> Leaving out this kind of cleanup in standalone command-line programs
> is fine, I think. It can make the code simpler - and it avoids the
> risk of doing it wrong, which can turn errors into crashes.
Hi Ian,
while I agree that this particular code would not be an issue,
consider that these tools are often taken as sample codes to be reused
in other contexts as well. As such, I think it should include the
close bits as well and exhibit all the "best practices" we would like
to see anyone else building tools for Xen.
Cheers,
Tamas
[xen-unstable-smoke test] 151454: tolerable all pass - PUSHED
flight 151454 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/151454/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen da53345dd5ff7d3a34e83587fd375c0b7722f46c baseline version: xen 88cfd062e8318dfeb67c7d2eb50b6cd224b0738a Last test of basis 151385 2020-06-26 18:00:39 Z2 days Testing same since 151454 2020-06-29 13:01:35 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Jan Beulich jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 88cfd062e8..da53345dd5 da53345dd5ff7d3a34e83587fd375c0b7722f46c -> smoke
Re: [PATCH] tools/configure: drop BASH configure variable
On 29.06.2020 14:05, Ian Jackson wrote:
> Jan Beulich writes ("Re: [PATCH] tools/configure: drop BASH configure
> variable"):
>> On 26.06.2020 19:00, Andrew Cooper wrote:
>>> diff --git a/xen/xsm/flask/policy/mkaccess_vector.sh
>>> b/xen/xsm/flask/policy/mkaccess_vector.sh
>>> old mode 100644
>>> new mode 100755
>>> diff --git a/xen/xsm/flask/policy/mkflask.sh
>>> b/xen/xsm/flask/policy/mkflask.sh
>>> old mode 100644
>>> new mode 100755
>>
>> ... this may or may not take effect on the file system the sources
>> are stored on.
>
> In what circumstances might this not take effect ?
When the file system is incapable of recording execute permissions?
It has been a common workaround for this in various projects that
I've worked with to use $(SHELL) to account for that, so the actual
permissions from the fs don't matter. (There may be mount options
to make everything executable on such file systems, but people may
be hesitant to use them.)
Jan
> IME all changes to files are properly replicated by git. Tarball
> distributions replicate the permissions of course.
>
> The only difficulty would be if this change were to be carried as a
> patch somewhere, by a downstream, but that seems unlikely, and can be
> avoided by that downstream not taking this patch.
>
> Ian.
>
RE: [PATCH] tools/configure: drop BASH configure variable
> -Original Message-
> From: Andrew Cooper
> Sent: 29 June 2020 14:51
> To: Ian Jackson
> Cc: Xen-devel ; Wei Liu ;
> Daniel De Graaf
> ; Paul Durrant
> Subject: Re: [PATCH] tools/configure: drop BASH configure variable
>
> On 29/06/2020 14:34, Ian Jackson wrote:
> > Andrew Cooper writes ("[PATCH] tools/configure: drop BASH configure
> > variable"):
> >> This is a weird variable to have in the first place. The only user of it
> >> is
> >> XSM's CONFIG_SHELL, which opencodes a fallback to sh, and the only two
> >> scripts
> >> run with this are shebang sh anyway, so don't need bash in the first place.
> > Thanks for this cleanup. I agree with the basic idea.
> >
> > However, did you run these scripts with dash, or review them, to check
> > for bashisms ?
>
> Yes, to all of the above.
>
> They are both very thin wrappers (doing some argument shuffling) around
> large AWK scripts.
>
> >> Make the mkflask.sh and mkaccess_vector.sh scripts executable, drop the
> >> CONFIG_SHELL, and drop the $BASH variable to prevent further use.
> > Since the build currently uses bash for these, a more neutral change
> > would be to change to #!/bin/bash at the same time.
>
> That will break FreeBSD, which has no `bash` in sight.
>
> >> RFC for 4.14. This is a cleanup to the build system.
> > I see this already has a release-ack. However, I would not have
> > recommended granting one at least on the basis of the description
> > above.
> >
> > I agree that this is cleanup. But the current situation is not buggy.
> > I'm not sure exactly what the release criteria are but ISTM that this
> > patch adds risk to the release rather than removing it.
>
> I agree that the current state of play isn't a major issue, but having
> ./configure check for bash is buggy for both uses.
>
Even if it is not buggy, it is pointless complexity since FreeBSD would clearly
have been broken all this time had there been bash-isms in the scripts.
Paul
> ~Andrew
Re: [PATCH] tools/configure: drop BASH configure variable
On 29/06/2020 14:34, Ian Jackson wrote:
> Andrew Cooper writes ("[PATCH] tools/configure: drop BASH configure
> variable"):
>> This is a weird variable to have in the first place. The only user of it is
>> XSM's CONFIG_SHELL, which opencodes a fallback to sh, and the only two
>> scripts
>> run with this are shebang sh anyway, so don't need bash in the first place.
> Thanks for this cleanup. I agree with the basic idea.
>
> However, did you run these scripts with dash, or review them, to check
> for bashisms ?
Yes, to all of the above.
They are both very thin wrappers (doing some argument shuffling) around
large AWK scripts.
>> Make the mkflask.sh and mkaccess_vector.sh scripts executable, drop the
>> CONFIG_SHELL, and drop the $BASH variable to prevent further use.
> Since the build currently uses bash for these, a more neutral change
> would be to change to #!/bin/bash at the same time.
That will break FreeBSD, which has no `bash` in sight.
>> RFC for 4.14. This is a cleanup to the build system.
> I see this already has a release-ack. However, I would not have
> recommended granting one at least on the basis of the description
> above.
>
> I agree that this is cleanup. But the current situation is not buggy.
> I'm not sure exactly what the release criteria are but ISTM that this
> patch adds risk to the release rather than removing it.
I agree that the current state of play isn't a major issue, but having
./configure check for bash is buggy for both uses.
~Andrew
[PATCH] tools/configure: drop BASH configure variable
Andrew Cooper writes ("[PATCH] tools/configure: drop BASH configure variable"):
> This is a weird variable to have in the first place. The only user of it is
> XSM's CONFIG_SHELL, which opencodes a fallback to sh, and the only two scripts
> run with this are shebang sh anyway, so don't need bash in the first place.
Thanks for this cleanup. I agree with the basic idea.
However, did you run these scripts with dash, or review them, to check
for bashisms ? It is not unusual to find scripts which need bash but
which mistaken have #!/bin/sh - especially if the usual arrangements
for running them always use bash anyway. So the presence of the
#!/bin/sh is only rather weak evidence that these scripts will be fine
when run with sh.
> Make the mkflask.sh and mkaccess_vector.sh scripts executable, drop the
> CONFIG_SHELL, and drop the $BASH variable to prevent further use.
Since the build currently uses bash for these, a more neutral change
would be to change to #!/bin/bash at the same time.
> RFC for 4.14. This is a cleanup to the build system.
I see this already has a release-ack. However, I would not have
recommended granting one at least on the basis of the description
above.
I agree that this is cleanup. But the current situation is not buggy.
I'm not sure exactly what the release criteria are but ISTM that this
patch adds risk to the release rather than removing it.
Thanks for your attention.
Ian.
Re: [PATCH] xsm: Drop trailing whitespace from build scripts
On 29/06/2020 09:25, Paul Durrant wrote: >> -Original Message- >> From: Xen-devel On Behalf Of Jan >> Beulich >> Sent: 29 June 2020 09:22 >> To: Andrew Cooper >> Cc: Xen-devel ; Daniel De Graaf >> >> Subject: Re: [PATCH] xsm: Drop trailing whitespace from build scripts >> >> On 26.06.2020 19:02, Andrew Cooper wrote: >>> Signed-off-by: Andrew Cooper >>> --- >>> CC: Daniel De Graaf >> Since we've not heard anything from Daniel in quite a while, just >> in case: >> Acked-by: Jan Beulich >> > This is pretty trivial cleanup so, if you want to put it in 4.14 consider > it... > > Release-acked-by: Paul Durrant Ok. In it goes. ~Andrew
Re: [PATCH] tools/configure: drop BASH configure variable
Jan Beulich writes ("Re: [PATCH] tools/configure: drop BASH configure
variable"):
> On 26.06.2020 19:00, Andrew Cooper wrote:
> > diff --git a/xen/xsm/flask/policy/mkaccess_vector.sh
> > b/xen/xsm/flask/policy/mkaccess_vector.sh
> > old mode 100644
> > new mode 100755
> > diff --git a/xen/xsm/flask/policy/mkflask.sh
> > b/xen/xsm/flask/policy/mkflask.sh
> > old mode 100644
> > new mode 100755
>
> ... this may or may not take effect on the file system the sources
> are stored on.
In what circumstances might this not take effect ?
IME all changes to files are properly replicated by git. Tarball
distributions replicate the permissions of course.
The only difficulty would be if this change were to be carried as a
patch somewhere, by a downstream, but that seems unlikely, and can be
avoided by that downstream not taking this patch.
Ian.
RE: [PATCH] x86/boot: Don't disable PV32 when XEN_SHSTK is compiled out
> -Original Message-
> From: Andrew Cooper
> Sent: 29 June 2020 11:31
> To: Xen-devel
> Cc: Andrew Cooper ; Jan Beulich
> ; Wei Liu ;
> Roger Pau Monné ; Paul Durrant
> Subject: [PATCH] x86/boot: Don't disable PV32 when XEN_SHSTK is compiled out
>
> There is no need to automatically disable PV32 support on SHSTK-capable
> hardware if Xen isn't actually using the feature.
>
> Signed-off-by: Andrew Cooper
> ---
> CC: Jan Beulich
> CC: Wei Liu
> CC: Roger Pau Monné
> CC: Paul Durrant
>
> For 4.14. Minor bugfix.
Release-acked-by: Paul Durrant
> ---
> xen/arch/x86/setup.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 2aa1cd50b8..c9b6af826d 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -95,7 +95,11 @@ unsigned long __initdata highmem_start;
> size_param("highmem-start", highmem_start);
> #endif
>
> +#ifdef CONFIG_XEN_SHSTK
> static bool __initdata opt_xen_shstk = true;
> +#else
> +#define opt_xen_shstk false
> +#endif
>
> static int __init parse_cet(const char *s)
> {
> --
> 2.11.0
Re: [PATCH] x86/boot: Don't disable PV32 when XEN_SHSTK is compiled out
On 29.06.2020 12:31, Andrew Cooper wrote: > There is no need to automatically disable PV32 support on SHSTK-capable > hardware if Xen isn't actually using the feature. > > Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich
Re: [PATCH v4 for-4.14 2/2] pvcalls: Document correctly and explicitely the padding for all arches
On 29/06/2020 12:22, Jan Beulich wrote: On 29.06.2020 12:03, Julien Grall wrote: On 29/06/2020 09:28, Jan Beulich wrote: On 27.06.2020 11:55, Julien Grall wrote: As an aside, the padding sadly cannot be mandated to be 0 as they are already present. So it is not going to be possible to use the padding for extending a command in the future. Why is the other adjustment fine to make due to still being tech preview, but this one wouldn't be for the same reason? This is mostly a left-over of the previous message. Although, I am not really inclined to address this myself any time soon. Sure, I didn't mean to indicate I might expect you to. But perhaps here the wording could be slightly changed as well? I am planning to remove the paragraph completely. Cheers, -- Julien Grall
Re: [PATCH v4 for-4.14 2/2] pvcalls: Document correctly and explicitely the padding for all arches
On 29.06.2020 12:03, Julien Grall wrote: > On 29/06/2020 09:28, Jan Beulich wrote: >> On 27.06.2020 11:55, Julien Grall wrote: >>> As an aside, the padding sadly cannot be mandated to be 0 as they are >>> already present. So it is not going to be possible to use the padding >>> for extending a command in the future. >> >> Why is the other adjustment fine to make due to still being tech >> preview, but this one wouldn't be for the same reason? > > This is mostly a left-over of the previous message. Although, I am not > really inclined to address this myself any time soon. Sure, I didn't mean to indicate I might expect you to. But perhaps here the wording could be slightly changed as well? Jan
[linux-linus test] 151441: regressions - FAIL
flight 151441 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/151441/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-arm64-arm64-libvirt-xsm 16 guest-start/debian.repeat fail REGR. vs. 151214 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 151214 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 151214 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 151214 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 151214 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 151214 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 151214 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 151214 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 151214 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-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-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 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-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass version targeted for testing: linux9ebcfadb0610322ac537dd7aa5d9cbc2b2894c68 baseline version: linux1b5044021070efa3259f3e9548dc35d1eb6aa844 Last test of basis 151214 2020-06-18 02:27:46 Z 11 days Failing since151236 2020-06-19 19:10:35 Z9 days 11 attempts Testing same since 151441 2020-06-28 23:39:48 Z0 days1 attempts 468 people touched revisions under test, not listing them all jobs: build-amd64-xsm
Re: [PATCH fsgsbase v2 4/4] x86/fsgsbase: Fix Xen PV support
On 29/06/2020 06:17, Jürgen Groß wrote:
> On 26.06.20 19:24, Andy Lutomirski wrote:
>> On Xen PV, SWAPGS doesn't work. Teach __rdfsbase_inactive() and
>> __wrgsbase_inactive() to use rdmsrl()/wrmsrl() on Xen PV. The Xen
>> pvop code will understand this and issue the correct hypercalls.
>>
>> Cc: Boris Ostrovsky
>> Cc: Juergen Gross
>> Cc: Stefano Stabellini
>> Cc: xen-devel@lists.xenproject.org
>> Signed-off-by: Andy Lutomirski
>> ---
>> arch/x86/kernel/process_64.c | 20 ++--
>> 1 file changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
>> index cb8e37d3acaa..457d02aa10d8 100644
>> --- a/arch/x86/kernel/process_64.c
>> +++ b/arch/x86/kernel/process_64.c
>> @@ -163,9 +163,13 @@ static noinstr unsigned long
>> __rdgsbase_inactive(void)
>> lockdep_assert_irqs_disabled();
>> - native_swapgs();
>> - gsbase = rdgsbase();
>> - native_swapgs();
>> + if (!static_cpu_has(X86_FEATURE_XENPV)) {
>> + native_swapgs();
>> + gsbase = rdgsbase();
>> + native_swapgs();
>> + } else {
>> + rdmsrl(MSR_KERNEL_GS_BASE, gsbase);
>> + }
>> return gsbase;
>> }
>> @@ -182,9 +186,13 @@ static noinstr void __wrgsbase_inactive(unsigned
>> long gsbase)
>> {
>> lockdep_assert_irqs_disabled();
>> - native_swapgs();
>> - wrgsbase(gsbase);
>> - native_swapgs();
>> + if (!static_cpu_has(X86_FEATURE_XENPV)) {
>> + native_swapgs();
>> + wrgsbase(gsbase);
>> + native_swapgs();
>> + } else {
>> + wrmsrl(MSR_KERNEL_GS_BASE, gsbase);
>> + }
>> }
>> /*
>>
>
> Another possibility would be to just do (I'm fine either way):
>
> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
> index acc49fa6a097..b78dd373adbf 100644
> --- a/arch/x86/xen/enlighten_pv.c
> +++ b/arch/x86/xen/enlighten_pv.c
> @@ -318,6 +318,8 @@ static void __init xen_init_capabilities(void)
> setup_clear_cpu_cap(X86_FEATURE_XSAVE);
> setup_clear_cpu_cap(X86_FEATURE_OSXSAVE);
> }
> +
> + setup_clear_cpu_cap(X86_FEATURE_FSGSBASE);
That will stop both userspace and Xen (side effect of the guest kernel's
CR4 choice) from using the instructions.
Even when the kernel is using the paravirt fastpath, its still Xen
actually taking the hit. MSR_{FS,GS}_BASE/SHADOW are thousands of
cycles to access, whereas {RD,WR}{FS,GS}BASE are a handful.
~Andrew
[PATCH] x86/boot: Don't disable PV32 when XEN_SHSTK is compiled out
There is no need to automatically disable PV32 support on SHSTK-capable
hardware if Xen isn't actually using the feature.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
CC: Paul Durrant
For 4.14. Minor bugfix.
---
xen/arch/x86/setup.c | 4
1 file changed, 4 insertions(+)
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 2aa1cd50b8..c9b6af826d 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -95,7 +95,11 @@ unsigned long __initdata highmem_start;
size_param("highmem-start", highmem_start);
#endif
+#ifdef CONFIG_XEN_SHSTK
static bool __initdata opt_xen_shstk = true;
+#else
+#define opt_xen_shstk false
+#endif
static int __init parse_cet(const char *s)
{
--
2.11.0
Re: Kernel requires x86-64 CPU, after modifying arch_shared_info struct
On Mon, Jun 29, 2020 at 09:56:43AM +, Jan Ruh wrote:
> > Did you try backing up your changes and seeing if the same happens?
>
> If backing up my changes everything works as expected.
>
> > Did you rebuild both the Xen hypervisor and the tools?
>
> Yes, I rebuild both Xen hypervisor and the tools.
>
> > Pasting your xl config file would be helpful in order to help debug.
>
> As requested my xl config:
xl parser will just ignore the ';', you can remove them.
> type="hvm"; extra="console=hvc0 earlyprintk=xen";
> kernel="/usr/lib/kernel/vmlinuz-domu";
> ramdisk="/usr/lib/kernel/initrd.img-domu";
> root="/dev/xvda2 ro";
> memory=1024;
> autoballoon="off";
autoballoon is not a xl.cfg option.
> xen_platform_pci=1;
> pae=1; acpi=1; apic=1;
All those are already enabled by default, no need to specify them
here.
> vcpus=1;
> name="vm1";
> disk=["file:domu.img,hda,w"];
> vif=["bridge=xenbr0"];
> vfb=["type=vnc,keymap=de"];
> vnclisten="192.168.2.4:0";
> boot="c";'
>
> > Posting your changes might also help spot something wonky.
>
> diff --git a/xen/include/public/arch-x86/xen.h
> b/xen/include/public/arch-x86/xen.h
> index 629cb2ba40..61c46504a5 100644
> --- a/xen/include/public/arch-x86/xen.h
> +++ b/xen/include/public/arch-x86/xen.h
> @@ -265,6 +265,14 @@ struct arch_shared_info {
> /* There's no room for this field in the generic structure. */
> uint32_t wc_sec_hi;
> #endif
> +
> +uint32_t st_version;
> +uint64_t time_sec;
> +uint64_t time_nsec;
> +uint64_t cycle_last;
> +uint32_t mult;
> +uint32_t shift;
> +
> };
> typedef struct arch_shared_info arch_shared_info_t;
>
> diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
> index c39fbe50a0..2782cb5127 100644
> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -1254,15 +1254,15 @@ void update_domain_synctime(struct domain *d)
This doesn't seem to exist in current Xen code, so I guess there are
further changes applied here?
> {
> uint32_t *st_version;
>
> +st_version = _info(d, arch.st_version);
> *st_version = version_update_begin(*st_version);
> smp_wmb();
>
> +shared_info(d, arch.mult)= global_time.mult;
> +shared_info(d, arch.shift) = global_time.shift;
> +shared_info(d, arch.cycle_last) = global_time.cycle_last;
> +shared_info(d, arch.time_sec)= global_time.time_sec;
> +shared_info(d, arch.time_nsec) = global_time.time_nsec;
>
> smp_wmb();
> *st_version = version_update_end(*st_version);
>
> diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
> index 72e7d33708..4b9ad0261b 100644
> --- a/xen/common/sysctl.c
> +++ b/xen/common/sysctl.c
> @@ -503,22 +503,22 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t)
> u_sysctl)
> }
> case XEN_SYSCTL_adjust_gtime:
> {
> +struct adjust_gtime *adjust_gtime = (struct adjust_gtime*)
> >u.adjust_gtime;
>
> +ret = do_adj_gtime(adjust_gtime);
Same with do_adj_gtime, I cannot find it in the current code, hence
I'm afraid it's impossible to tell what it's actually doing.
> +
> copyback = 1;
>
> break;
>
> diff --git a/tools/include/xen-foreign/reference.size
> b/tools/include/xen-foreign/reference.size
> index bb2ada32fb..9afd11e5fa 100644
> --- a/tools/include/xen-foreign/reference.size
> +++ b/tools/include/xen-foreign/reference.size
> @@ -9,6 +9,6 @@ vcpu_guest_context| 344 34428005168
> arch_vcpu_info| 0 0 24 16
> vcpu_time_info| 32 32 32 32
> vcpu_info | 48 48 64 64
> -arch_shared_info | 0 0 28 48
> +arch_shared_info | 0 0 64 88
Aren't you missing a line below that contains the shared_info size,
and that also need to be updated (since arch_shared_info is contained
in shared_info)?
>
>
> global_time is a static struct in time.c, no existing Xen code is changed,
> just functions added that are being called from the sysctl adjust_gtime.
>
> Further tests show that in /tools/libxc/xc_dom_binloader.c:
> xc_dom_parse_bin_kernel xc sets the dom->guest_type to "xen-3.0-x86_32"
> instead of "xen-3.0-x86_64". I also cannot see though how it can be connected
> to my change to arch_shared_info.
Hm, I think guest type should be hvm-3.0-x86_32, as xen-* are PV guest
types, and you are trying to boot a HVM guest.
Anyway I'm not familiar with HVM direct kernel boot, so this might
have no effect here.
Are you sure the type is set to "xen-3.0-x86_64" prior to your
changes?
Maybe it would be worth to also paste the output of 'xl -vvv create
...'.
> Sorry for the banner, I always forget that the mail client adds that thing, I
> hope it doesn't do it again.
I'm afraid it's still appended to this email, see below.
Roger.
> CONFIDENTIALITY: The contents of this e-mail are
Re: [PATCH v4 for-4.14 2/2] pvcalls: Document correctly and explicitely the padding for all arches
Hi Jan, On 29/06/2020 09:28, Jan Beulich wrote: On 27.06.2020 11:55, Julien Grall wrote: From: Julien Grall The specification of pvcalls suggests there is padding for 32-bit x86 at the end of most the structure. However, they are not described in in the public header. Because of that all the structures would be 32-bit aligned and not 64-bit aligned for 32-bit x86. The added padding doesn't change the alignment. It's sizeof() which gets corrected this way. I will update the commit message. For all the other architectures supported (Arm and 64-bit x86), the structure are aligned to 64-bit because they contain uint64_t field. Therefore all the structures contain implicit padding. Given the specification is authoriitative, the padding will the same for Nit: ... will be the same ... Ok. the all architectures. The potential breakage of compatibility is ought Nit: Drop "is". Ok. to be fine as pvcalls is still a tech preview. As an aside, the padding sadly cannot be mandated to be 0 as they are already present. So it is not going to be possible to use the padding for extending a command in the future. Why is the other adjustment fine to make due to still being tech preview, but this one wouldn't be for the same reason? This is mostly a left-over of the previous message. Although, I am not really inclined to address this myself any time soon. Cheers, -- Julien Grall
Aw: Re: Questions about PVH memory layout
Hi Roger, thank you very much for your help. Further replies are inline. > Gesendet: Montag, 29. Juni 2020 um 10:57 Uhr > Von: "Roger Pau Monné" > An: joshua_pe...@web.de > Cc: xen-devel@lists.xenproject.org > Betreff: Re: Questions about PVH memory layout > > > The other > > thing is, the first 512 pages at the beginning of the address space are > > mapped linearly, which usually leads to them being mapped as a single > > 2MB pages. But there is this one page at 0x1000 that sticks out > > completely. By that I mean (to make things more concrete), in my PVH > > guest the page at 0x maps to 0x13C20, 0x2000 maps to > > 0x13C202000, 0x3000 maps to 0x13C203000, etc. But 0x1000 maps > > to 0xB8DBD000, which seems very odd to me (at least from simply looking > > at the addresses). > > Are you booting some OS on the guest before dumping the memory map? > Keep in mind guest have the ability to modify the physmap, either by > mapping Xen shared areas (like the shared info page) or just by using > ballooning in order to poke holes into it (which can be populated > later). It's either that or some kind of bug/missoptimization in > meminit_hvm (also part of tools/libxc/xc_dom_x86.c). Yes, my bad. I'm booting into Arch Linux. This must have been lost while I was editing my e-mail. > Can you check if this 'weird' mapping at 0x1000 is also present if you > boot the guest with 'xl create -p foo.cfg'? (that will prevent the > guests from running, so that you can get the memory map before the > guest has modified it in any way). Yeah, starting with the "-p" flag does get rid of this 'weird' mapping. Thank you again. This cleared up a bunch of things. Best regards, Peter
AW: Kernel requires x86-64 CPU, after modifying arch_shared_info struct
> Did you try backing up your changes and seeing if the same happens?
If backing up my changes everything works as expected.
> Did you rebuild both the Xen hypervisor and the tools?
Yes, I rebuild both Xen hypervisor and the tools.
> Pasting your xl config file would be helpful in order to help debug.
As requested my xl config:
type="hvm"; extra="console=hvc0 earlyprintk=xen";
kernel="/usr/lib/kernel/vmlinuz-domu";
ramdisk="/usr/lib/kernel/initrd.img-domu";
root="/dev/xvda2 ro";
memory=1024;
autoballoon="off";
xen_platform_pci=1;
pae=1; acpi=1; apic=1; vcpus=1;
name="vm1";
disk=["file:domu.img,hda,w"];
vif=["bridge=xenbr0"];
vfb=["type=vnc,keymap=de"];
vnclisten="192.168.2.4:0";
boot="c";'
> Posting your changes might also help spot something wonky.
diff --git a/xen/include/public/arch-x86/xen.h
b/xen/include/public/arch-x86/xen.h
index 629cb2ba40..61c46504a5 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -265,6 +265,14 @@ struct arch_shared_info {
/* There's no room for this field in the generic structure. */
uint32_t wc_sec_hi;
#endif
+
+uint32_t st_version;
+uint64_t time_sec;
+uint64_t time_nsec;
+uint64_t cycle_last;
+uint32_t mult;
+uint32_t shift;
+
};
typedef struct arch_shared_info arch_shared_info_t;
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index c39fbe50a0..2782cb5127 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1254,15 +1254,15 @@ void update_domain_synctime(struct domain *d)
{
uint32_t *st_version;
+st_version = _info(d, arch.st_version);
*st_version = version_update_begin(*st_version);
smp_wmb();
+shared_info(d, arch.mult)= global_time.mult;
+shared_info(d, arch.shift) = global_time.shift;
+shared_info(d, arch.cycle_last) = global_time.cycle_last;
+shared_info(d, arch.time_sec)= global_time.time_sec;
+shared_info(d, arch.time_nsec) = global_time.time_nsec;
smp_wmb();
*st_version = version_update_end(*st_version);
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index 72e7d33708..4b9ad0261b 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -503,22 +503,22 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t)
u_sysctl)
}
case XEN_SYSCTL_adjust_gtime:
{
+struct adjust_gtime *adjust_gtime = (struct adjust_gtime*)
>u.adjust_gtime;
+ret = do_adj_gtime(adjust_gtime);
+
copyback = 1;
break;
diff --git a/tools/include/xen-foreign/reference.size
b/tools/include/xen-foreign/reference.size
index bb2ada32fb..9afd11e5fa 100644
--- a/tools/include/xen-foreign/reference.size
+++ b/tools/include/xen-foreign/reference.size
@@ -9,6 +9,6 @@ vcpu_guest_context| 344 34428005168
arch_vcpu_info| 0 0 24 16
vcpu_time_info| 32 32 32 32
vcpu_info | 48 48 64 64
-arch_shared_info | 0 0 28 48
+arch_shared_info | 0 0 64 88
global_time is a static struct in time.c, no existing Xen code is changed, just
functions added that are being called from the sysctl adjust_gtime.
Further tests show that in /tools/libxc/xc_dom_binloader.c:
xc_dom_parse_bin_kernel xc sets the dom->guest_type to "xen-3.0-x86_32" instead
of "xen-3.0-x86_64". I also cannot see though how it can be connected to my
change to arch_shared_info.
Sorry for the banner, I always forget that the mail client adds that thing, I
hope it doesn't do it again.
Jan
CONFIDENTIALITY: The contents of this e-mail are confidential and intended only
for the above addressee(s). If you are not the intended recipient, or the
person responsible for delivering it to the intended recipient, copying or
delivering it to anyone else or using it in any unauthorized manner is
prohibited and may be unlawful. If you receive this e-mail by mistake, please
notify the sender and the systems administrator at straym...@tttech.com
immediately.
Re: [PATCH v3 4/7] x86/vmx: add do_vmtrace_op
- 23 cze 2020 o 13:54, Andrew Cooper andrew.coop...@citrix.com napisał(a): > Overall, the moving parts of this series needs to split out into rather > more patches. > > First, in patch 3, the hvm_funcs.pt_supported isn't the place for that > to live. You want a global "bool vmtrace_supported" in common/domain.c > which vmx_init_vmcs_config() sets, and the ARM code can set in the > future when CoreSight is added. > > Next, you want a patch in isolation which adds vmtrace_pt_size (or > whatever it ends up being) to createdomain, where all > allocation/deallocation logic lives in common/domain.c. The spinlock > (if its needed, but I don't think it is) wants initialising early in > domain_create(), alongside d->pbuf_lock, and you also need an extra > clause in sanitise_domain_config() which rejects a vmtrace setting if > vmtrace isn't supported. You'll need to put the struct page_info * > pointer to the memory allocation in struct vcpu, and adjust the vcpu > create/destroy logic appropriately. > > Next, you want a patch doing the acquire resource logic for userspace to > map the buffers. > > Next, you want a patch to introduce a domctl with the various runtime > enable/disable settings which were in an hvmop here. > > Next, you want a patch to do the VMX plumbing, both at create, and runtime. > > This ought to lay the logic out in a way which is extendable to x86 PV > guests and ARM CoreSight, and oughtn't to explode when creating guests > on non-Intel hardware. > > Thanks, > > ~Andrew Thanks for your review, I'm almost done addressing all these remarks. I've converted HVMOP to DOMCTL and splitted patches to smaller pieces. I will send v4 soon. Best regards, Michał Leszczyński
Re: Suspend/hibernate not working on Dell XPS 15 9500 laptop
Completely a shot in the dark, but have you tried with legacy boot (BIOS) instead of UEFI? To try to rule out what might cause the issues. From the BIOS: "Legacy Boot mode is not supported on this platform." This is my punishment for buying a brand new laptop model for Linux... -Trevor
Re: Suspend/hibernate not working on Dell XPS 15 9500 laptop
I don't suppose you have a serial port on this laptop? I ask because the serial log (and the possibility to issue debug keys) are about the only thing debugging-wise that may help here, short of someone noticing the underlying problem by code inspection. Afraid not, it's hyper-modern; just 3 USB-C ports. It looks like identical hardware in the Dell Precision 5550 model, in case anybody gets access to either and wants to test. Do you have any indication of the laptop at least partly waking up again, e.g. from some LED or other indicator activity? None at all, no. Could be missing the keyboard events entirely, or getting them and failing to wake. I've tried built-in keyboard, external keyboard, and closing/opening the lid. When using the /sys/power/pm_test commands, does that actually do anything in a Xen environment, or is that being consumed by dom0 and not triggering the real VMM low-power paths? Is there any similar debug mechanism for the hypervisor? Let me know if there is any useful information I can extract. I'm happy to build patched kernels if it will help. Thanks, -Trevor
[ovmf test] 151444: all pass - PUSHED
flight 151444 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/151444/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 0060e0a694f3f249c3ec081b0e61287c36f64ebb baseline version: ovmf 654dc3ed852aafa126e9d539b7002db348dd6eb0 Last test of basis 151401 2020-06-27 09:09:22 Z2 days Testing same since 151444 2020-06-29 02:39:29 Z0 days1 attempts People who touched revisions under test: Dong, Eric Eric Dong jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/osstest/ovmf.git 654dc3ed85..0060e0a694 0060e0a694f3f249c3ec081b0e61287c36f64ebb -> xen-tested-master
Re: Suspend/hibernate not working on Dell XPS 15 9500 laptop
On Sat, Jun 27, 2020 at 08:35:27AM +, Trevor Bentley wrote: > I asked on #xen on Freenode and was requested to post here. > > Summary: Under Xen, both suspend and hibernate operations put the laptop > into some sort of unrecoverable low-power mode, and a force power-off is > required. > > Environment: > - Dell XPS 15 9500, BIOS 1.0.1 (this is a new 2020 model) > - Intel i7-10750H > - Intel i915 + Nvidia GTX 1650 Ti Mobile > - Arch linux (clean install) > - Linux kernels 5.7.5, 5.7.6 tested > - i915 driver loaded, no nvidia drivers loaded (nouveau blacklisted) > - Xen 4.13.1, 4.14.0-rc3 tested > - UEFI, GRUB2 bootloader, LUKS-encrypted /boot, /root, and swap > (unencrypted /efi with GRUB stub loader) Completely a shot in the dark, but have you tried with legacy boot (BIOS) instead of UEFI? To try to rule out what might cause the issues. Roger.
Re: Kernel requires x86-64 CPU, after modifying arch_shared_info struct
On Mon, Jun 29, 2020 at 07:43:43AM +, Jan Ruh wrote: > Hi Xen Dev Community, > > > I ran into an issue when implementing a prototype for a new > paravirtualized clock for x86-64 hosts. I extended the > arch_shared_info struct by 6 fields totaling at 36 bytes. I updated > the xen-foreign/references.size to represent the new size of the > arch_shared_info struct. The fields are correctly updated in Xen and > I am also able to read the correct information stored from dom0. > However, if I try to start a hvm VM with pvh extensions it does not > boot telling me that "This kernel requires an x86-64 CPU, but only > detected an i686 CPU.". Did you try backing up your changes and seeing if the same happens? Did you rebuild both the Xen hypervisor and the tools? > I have rebuild my Linux domU kernel just as > the dom0 kernel and everything should be compatible. To me this > looks like Xen or libxc does some compatibility checks before > booting up my HVM domU and decides to downgrade the detectable CPU > due to some issue that I am not aware of. Do I miss something? Pasting your xl config file would be helpful in order to help debug. > Is my > approach to extend the arch_shared_info wrong in the first place? Doesn't look directly related to a change in arch_shared_info IMO, but it's hard to tell without having more info. Posting your changes might also help spot something wonky. > CONFIDENTIALITY: The contents of this e-mail are confidential and > intended only for the above addressee(s). If you are not the > intended recipient, or the person responsible for delivering it to > the intended recipient, copying or delivering it to anyone else or > using it in any unauthorized manner is prohibited and may be > unlawful. If you receive this e-mail by mistake, please notify the > sender and the systems administrator at straym...@tttech.com > immediately. Nit: posting confidentiality banners to a public mailing lists messages is kind of award, as the emails are publicly available for anyone to read, even those that are not recipients of xen-devel, ie: https://lists.xenproject.org/archives/html/xen-devel/2020-06/msg01868.html Roger.
Re: Questions about PVH memory layout
On Sun, Jun 28, 2020 at 08:58:14AM +0200, joshua_pe...@web.de wrote: > Hello everyone, > > I hope this is the right forum for these kinds of questions (the website > states no "technical support queries"; I'm not sure if this qualifies). > If not, sorry for the disturbance; just simply direct me elsewhere then. > > Anyway, I'm currently trying to get into how Xen works in detail, so > for one I've been reading a lot of code, but also I dumped the P2M table > of my PVH guest to get a feel for how things are layed out in memory. I > mean there is the usual stuff, such as lots of RAM, and the APIC is > mapped at 0xFEE0 and the APCI tables at 0xFC00 onwards. But two > things stuck out to me, which for the life of me I couldn't figure out > from just reading the code. The first one is, there are a few pages at > the end of the 32bit address space (from 0xFEFF8000 to 0xFEFFF000), > which according to the E820 is declared simply as "reserved". Those are the HVM special pages, which are used for various Xen specific things, like the shared memory ring for the PV console. They are setup in tools/libxc/xc_dom_x86.c (see SPECIALPAGE_*). > The other > thing is, the first 512 pages at the beginning of the address space are > mapped linearly, which usually leads to them being mapped as a single > 2MB pages. But there is this one page at 0x1000 that sticks out > completely. By that I mean (to make things more concrete), in my PVH > guest the page at 0x maps to 0x13C20, 0x2000 maps to > 0x13C202000, 0x3000 maps to 0x13C203000, etc. But 0x1000 maps > to 0xB8DBD000, which seems very odd to me (at least from simply looking > at the addresses). Are you booting some OS on the guest before dumping the memory map? Keep in mind guest have the ability to modify the physmap, either by mapping Xen shared areas (like the shared info page) or just by using ballooning in order to poke holes into it (which can be populated later). It's either that or some kind of bug/missoptimization in meminit_hvm (also part of tools/libxc/xc_dom_x86.c). Can you check if this 'weird' mapping at 0x1000 is also present if you boot the guest with 'xl create -p foo.cfg'? (that will prevent the guests from running, so that you can get the memory map before the guest has modified it in any way). > My initial guess was that this is some bootloader > related stuff, but Google didn't show up any info related to that > memory area, and most of the x86/PC boot stuff seems to happen below > the 0x1000 mark. If you are booting with pygrub there's no bootloader at all, so whatever is happening is either done by the toolstack or the OS you are loading. Roger.
Re: [PATCH] tools/configure: drop BASH configure variable
On 26.06.2020 19:00, Andrew Cooper wrote: > @@ -24,14 +20,14 @@ extra-y += $(ALL_H_FILES) > > mkflask := policy/mkflask.sh > quiet_cmd_mkflask = MKFLASK $@ > -cmd_mkflask = $(CONFIG_SHELL) $(mkflask) $(AWK) include $(FLASK_H_DEPEND) > +cmd_mkflask = $(mkflask) $(AWK) include $(FLASK_H_DEPEND) This and ... > $(subst include/,%/,$(FLASK_H_FILES)): $(FLASK_H_DEPEND) $(mkflask) FORCE > $(call if_changed,mkflask) > > mkaccess := policy/mkaccess_vector.sh > quiet_cmd_mkaccess = MKACCESS VECTOR $@ > -cmd_mkaccess = $(CONFIG_SHELL) $(mkaccess) $(AWK) $(AV_H_DEPEND) > +cmd_mkaccess = $(mkaccess) $(AWK) $(AV_H_DEPEND) ... this should still use $(SHELL) though, as ... > diff --git a/xen/xsm/flask/policy/mkaccess_vector.sh > b/xen/xsm/flask/policy/mkaccess_vector.sh > old mode 100644 > new mode 100755 > diff --git a/xen/xsm/flask/policy/mkflask.sh b/xen/xsm/flask/policy/mkflask.sh > old mode 100644 > new mode 100755 ... this may or may not take effect on the file system the sources are stored on. Jan
Re: Suspend/hibernate not working on Dell XPS 15 9500 laptop
On 27.06.2020 10:35, Trevor Bentley wrote: > Please let me know if you have any suggestions to try, or if you need > any extra information for debugging. I don't suppose you have a serial port on this laptop? I ask because the serial log (and the possibility to issue debug keys) are about the only thing debugging-wise that may help here, short of someone noticing the underlying problem by code inspection. Do you have any indication of the laptop at least partly waking up again, e.g. from some LED or other indicator activity? Jan
Re: [PATCH v4 for-4.14 2/2] pvcalls: Document correctly and explicitely the padding for all arches
On 27.06.2020 11:55, Julien Grall wrote: > From: Julien Grall > > The specification of pvcalls suggests there is padding for 32-bit x86 > at the end of most the structure. However, they are not described in > in the public header. > > Because of that all the structures would be 32-bit aligned and not > 64-bit aligned for 32-bit x86. The added padding doesn't change the alignment. It's sizeof() which gets corrected this way. > For all the other architectures supported (Arm and 64-bit x86), the > structure are aligned to 64-bit because they contain uint64_t field. > Therefore all the structures contain implicit padding. > > Given the specification is authoriitative, the padding will the same for Nit: ... will be the same ... > the all architectures. The potential breakage of compatibility is ought Nit: Drop "is". > to be fine as pvcalls is still a tech preview. > > As an aside, the padding sadly cannot be mandated to be 0 as they are > already present. So it is not going to be possible to use the padding > for extending a command in the future. Why is the other adjustment fine to make due to still being tech preview, but this one wouldn't be for the same reason? Jan
RE: [PATCH] xsm: Drop trailing whitespace from build scripts
> -Original Message- > From: Xen-devel On Behalf Of Jan > Beulich > Sent: 29 June 2020 09:22 > To: Andrew Cooper > Cc: Xen-devel ; Daniel De Graaf > > Subject: Re: [PATCH] xsm: Drop trailing whitespace from build scripts > > On 26.06.2020 19:02, Andrew Cooper wrote: > > Signed-off-by: Andrew Cooper > > --- > > CC: Daniel De Graaf > > Since we've not heard anything from Daniel in quite a while, just > in case: > Acked-by: Jan Beulich > This is pretty trivial cleanup so, if you want to put it in 4.14 consider it... Release-acked-by: Paul Durrant > > Jan
Re: [PATCH] xsm: Drop trailing whitespace from build scripts
On 26.06.2020 19:02, Andrew Cooper wrote: > Signed-off-by: Andrew Cooper > --- > CC: Daniel De Graaf Since we've not heard anything from Daniel in quite a while, just in case: Acked-by: Jan Beulich Jan
Kernel requires x86-64 CPU, after modifying arch_shared_info struct
Hi Xen Dev Community, I ran into an issue when implementing a prototype for a new paravirtualized clock for x86-64 hosts. I extended the arch_shared_info struct by 6 fields totaling at 36 bytes. I updated the xen-foreign/references.size to represent the new size of the arch_shared_info struct. The fields are correctly updated in Xen and I am also able to read the correct information stored from dom0. However, if I try to start a hvm VM with pvh extensions it does not boot telling me that "This kernel requires an x86-64 CPU, but only detected an i686 CPU.". I have rebuild my Linux domU kernel just as the dom0 kernel and everything should be compatible. To me this looks like Xen or libxc does some compatibility checks before booting up my HVM domU and decides to downgrade the detectable CPU due to some issue that I am not aware of. Do I miss something? Is my approach to extend the arch_shared_info wrong in the first place? I am really thankful for some advice or pointers what is happening here. Best Jan CONFIDENTIALITY: The contents of this e-mail are confidential and intended only for the above addressee(s). If you are not the intended recipient, or the person responsible for delivering it to the intended recipient, copying or delivering it to anyone else or using it in any unauthorized manner is prohibited and may be unlawful. If you receive this e-mail by mistake, please notify the sender and the systems administrator at straym...@tttech.com immediately.
RE: [PATCH v2 2/2] optee: allow plain TMEM buffers with NULL address
> -Original Message-
> From: Julien Grall
> Sent: 26 June 2020 18:54
> To: Stefano Stabellini ; p...@xen.org
> Cc: Volodymyr Babchuk ;
> xen-devel@lists.xenproject.org; op-
> t...@lists.trustedfirmware.org
> Subject: Re: [PATCH v2 2/2] optee: allow plain TMEM buffers with NULL address
>
> (using paul xen.org's email)
>
Thanks. Avoids annoying warning banners :-)
> Hi,
>
> Apologies for the late answer.
>
> On 23/06/2020 22:09, Stefano Stabellini wrote:
> > On Tue, 23 Jun 2020, Julien Grall wrote:
> >> On 23/06/2020 03:49, Volodymyr Babchuk wrote:
> >>>
> >>> Hi Stefano,
> >>>
> >>> Stefano Stabellini writes:
> >>>
> On Fri, 19 Jun 2020, Volodymyr Babchuk wrote:
> > Trusted Applications use popular approach to determine required size
> > of buffer: client provides a memory reference with the NULL pointer to
> > a buffer. This is so called "Null memory reference". TA updates the
> > reference with the required size and returns it back to the
> > client. Then client allocates buffer of needed size and repeats the
> > operation.
> >
> > This behavior is described in TEE Client API Specification, paragraph
> > 3.2.5. Memory References.
> >
> > OP-TEE represents this null memory reference as a TMEM parameter with
> > buf_ptr = 0x0. This is the only case when we should allow TMEM
> > buffer without the OPTEE_MSG_ATTR_NONCONTIG flag. This also the
> > special case for a buffer with OPTEE_MSG_ATTR_NONCONTIG flag.
> >
> > This could lead to a potential issue, because IPA 0x0 is a valid
> > address, but OP-TEE will treat it as a special case. So, care should
> > be taken when construction OP-TEE enabled guest to make sure that such
> > guest have no memory at IPA 0x0 and none of its memory is mapped at PA
> > 0x0.
> >
> > Signed-off-by: Volodymyr Babchuk
> > ---
> >
> > Changes from v1:
> >- Added comment with TODO about possible PA/IPA 0x0 issue
> >- The same is described in the commit message
> >- Added check in translate_noncontig() for the NULL ptr buffer
> >
> > ---
> >xen/arch/arm/tee/optee.c | 27 ---
> >1 file changed, 24 insertions(+), 3 deletions(-)
> >
> > diff --git a/xen/arch/arm/tee/optee.c b/xen/arch/arm/tee/optee.c
> > index 6963238056..70bfef7e5f 100644
> > --- a/xen/arch/arm/tee/optee.c
> > +++ b/xen/arch/arm/tee/optee.c
> > @@ -215,6 +215,15 @@ static bool optee_probe(void)
> >return true;
> >}
> >+/*
> > + * TODO: There is a potential issue with guests that either have RAM
> > + * at IPA of 0x0 or some of theirs memory is mapped at PA 0x0. This is
> ^ their
>
> > + * because PA of 0x0 is considered as NULL pointer by OP-TEE. It will
> > + * not be able to map buffer with such pointer to TA address space, or
> > + * use such buffer for communication with the guest. We either need to
> > + * check that guest have no such mappings or ensure that OP-TEE
> > + * enabled guest will not be created with such mappings.
> > + */
> >static int optee_domain_init(struct domain *d)
> >{
> >struct arm_smccc_res resp;
> > @@ -725,6 +734,15 @@ static int translate_noncontig(struct optee_domain
> > *ctx,
> >uint64_t next_page_data;
> >} *guest_data, *xen_data;
> >+/*
> > + * Special case: buffer with buf_ptr == 0x0 is considered as NULL
> > + * pointer by OP-TEE. No translation is needed. This can lead to
> > + * an issue as IPA 0x0 is a valid address for Xen. See the comment
> > + * near optee_domain_init()
> > + */
> > +if ( !param->u.tmem.buf_ptr )
> > +return 0;
>
> Given that today it is not possible for this to happen, it could even be
> an ASSERT. But I think I would just return an error, maybe -EINVAL?
> >>>
> >>> Hmm, looks like my comment is somewhat misleading :(
> >>
> >> How about the following comment:
> >>
> >> We don't want to translate NULL (0) as it can be used by the guest to fetch
> >> the size of the buffer to allocate. This behavior depends on TA, but there
> >> is
> >> a guarantee that OP-TEE will not try to map it (see more details on top of
> >> optee_domain_init()).
> >>
> >>>
> >>> What I mean, is that param->u.tmem.buf_ptr == 0 is the normal situation.
> >>> This is the special case, when OP-TEE treats this buffer as a NULL. So
> >>> we are doing nothing there. Thus, "return 0".
> >>>
> >>> But, as Julien pointed out, we can have machine where 0x0 is the valid
> >>> memory address and there is a chance, that some guest will use it as a
> >>> pointer to buffer.
> >>>
> Aside from this, and the small grammar issue, everything else looks fine
> to me.
>
> Let's wait for Julien's reply, but if this is the
RE: [PATCH] tools/configure: drop BASH configure variable
> -Original Message- > From: Andrew Cooper > Sent: 26 June 2020 18:01 > To: Xen-devel > Cc: Andrew Cooper ; Ian Jackson > ; Wei Liu > ; Daniel De Graaf ; Paul Durrant > > Subject: [PATCH] tools/configure: drop BASH configure variable > > This is a weird variable to have in the first place. The only user of it is > XSM's CONFIG_SHELL, which opencodes a fallback to sh, and the only two scripts > run with this are shebang sh anyway, so don't need bash in the first place. > > Make the mkflask.sh and mkaccess_vector.sh scripts executable, drop the > CONFIG_SHELL, and drop the $BASH variable to prevent further use. > > Signed-off-by: Andrew Cooper > --- > CC: Ian Jackson > CC: Wei Liu > CC: Daniel De Graaf > CC: Paul Durrant > > ./autogen.sh needs rerunning on commit. > > RFC for 4.14. This is a cleanup to the build system. > > There is a separate check for [BASH] in the configure script, which is > checking the requirement for the Linux hotplug scripts. Really, that is a > runtime requirement not a build time requirement, and it is rude to require > bash in a build environment on this basis. IMO, that check wants dropping as > well. That sounds quite reasonable. Release-acked-by: Paul Durrant > --- > config/Tools.mk.in | 1 - > tools/configure.ac | 1 - > xen/xsm/flask/Makefile | 8 ++-- > xen/xsm/flask/policy/mkaccess_vector.sh | 0 > xen/xsm/flask/policy/mkflask.sh | 0 > 5 files changed, 2 insertions(+), 8 deletions(-) > mode change 100644 => 100755 xen/xsm/flask/policy/mkaccess_vector.sh > mode change 100644 => 100755 xen/xsm/flask/policy/mkflask.sh > > diff --git a/config/Tools.mk.in b/config/Tools.mk.in > index 23df47af8d..48bd9ab731 100644 > --- a/config/Tools.mk.in > +++ b/config/Tools.mk.in > @@ -12,7 +12,6 @@ PYTHON := @PYTHON@ > PYTHON_PATH := @PYTHONPATH@ > PY_NOOPT_CFLAGS := @PY_NOOPT_CFLAGS@ > PERL:= @PERL@ > -BASH:= @BASH@ > XGETTTEXT := @XGETTEXT@ > AS86:= @AS86@ > LD86:= @LD86@ > diff --git a/tools/configure.ac b/tools/configure.ac > index 9d126b7a14..6614a4f130 100644 > --- a/tools/configure.ac > +++ b/tools/configure.ac > @@ -297,7 +297,6 @@ AC_ARG_VAR([PYTHON], [Path to the Python parser]) > AC_ARG_VAR([PERL], [Path to Perl parser]) > AC_ARG_VAR([BISON], [Path to Bison parser generator]) > AC_ARG_VAR([FLEX], [Path to Flex lexical analyser generator]) > -AC_ARG_VAR([BASH], [Path to bash shell]) > AC_ARG_VAR([XGETTEXT], [Path to xgetttext tool]) > AC_ARG_VAR([AS86], [Path to as86 tool]) > AC_ARG_VAR([LD86], [Path to ld86 tool]) > diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile > index 07f36d075d..ba8f31a02c 100644 > --- a/xen/xsm/flask/Makefile > +++ b/xen/xsm/flask/Makefile > @@ -8,10 +8,6 @@ CFLAGS-y += -I./include > > AWK = awk > > -CONFIG_SHELL := $(shell if [ -x "$$BASH" ]; then echo $$BASH; \ > - else if [ -x /bin/bash ]; then echo /bin/bash; \ > - else echo sh; fi ; fi) > - > FLASK_H_DEPEND = policy/security_classes policy/initial_sids > AV_H_DEPEND = policy/access_vectors > > @@ -24,14 +20,14 @@ extra-y += $(ALL_H_FILES) > > mkflask := policy/mkflask.sh > quiet_cmd_mkflask = MKFLASK $@ > -cmd_mkflask = $(CONFIG_SHELL) $(mkflask) $(AWK) include $(FLASK_H_DEPEND) > +cmd_mkflask = $(mkflask) $(AWK) include $(FLASK_H_DEPEND) > > $(subst include/,%/,$(FLASK_H_FILES)): $(FLASK_H_DEPEND) $(mkflask) FORCE > $(call if_changed,mkflask) > > mkaccess := policy/mkaccess_vector.sh > quiet_cmd_mkaccess = MKACCESS VECTOR $@ > -cmd_mkaccess = $(CONFIG_SHELL) $(mkaccess) $(AWK) $(AV_H_DEPEND) > +cmd_mkaccess = $(mkaccess) $(AWK) $(AV_H_DEPEND) > > $(subst include/,%/,$(AV_H_FILES)): $(AV_H_DEPEND) $(mkaccess) FORCE > $(call if_changed,mkaccess) > diff --git a/xen/xsm/flask/policy/mkaccess_vector.sh > b/xen/xsm/flask/policy/mkaccess_vector.sh > old mode 100644 > new mode 100755 > diff --git a/xen/xsm/flask/policy/mkflask.sh b/xen/xsm/flask/policy/mkflask.sh > old mode 100644 > new mode 100755 > -- > 2.11.0
RE: [PATCH v4 for-4.14 2/2] pvcalls: Document correctly and explicitely the padding for all arches
> -Original Message- > From: Jürgen Groß > Sent: 27 June 2020 12:54 > To: Julien Grall ; xen-devel@lists.xenproject.org > Cc: p...@xen.org; Julien Grall ; Andrew Cooper > ; George > Dunlap ; Ian Jackson ; > Jan Beulich > ; Stefano Stabellini ; Wei Liu > > Subject: Re: [PATCH v4 for-4.14 2/2] pvcalls: Document correctly and > explicitely the padding for all > arches > > On 27.06.20 11:55, Julien Grall wrote: > > From: Julien Grall > > > > The specification of pvcalls suggests there is padding for 32-bit x86 > > at the end of most the structure. However, they are not described in > > in the public header. > > > > Because of that all the structures would be 32-bit aligned and not > > 64-bit aligned for 32-bit x86. > > > > For all the other architectures supported (Arm and 64-bit x86), the > > structure are aligned to 64-bit because they contain uint64_t field. > > Therefore all the structures contain implicit padding. > > > > Given the specification is authoriitative, the padding will the same for > > s/authoriitative/authoritative/ > > > the all architectures. The potential breakage of compatibility is ought > > s/the// > > > to be fine as pvcalls is still a tech preview. > > > > As an aside, the padding sadly cannot be mandated to be 0 as they are > > already present. So it is not going to be possible to use the padding > > for extending a command in the future. > > > > Signed-off-by: Julien Grall > > With above fixed: > > Reviewed-by: Juergen Gross > Release-acked-by: Paul Durrant > > Juergen
RE: [PATCH v4 for-4.14 1/2] pvcalls: Clearly spell out that the header is just a reference
> -Original Message- > From: Jürgen Groß > Sent: 27 June 2020 12:55 > To: Julien Grall ; xen-devel@lists.xenproject.org > Cc: p...@xen.org; Julien Grall > Subject: Re: [PATCH v4 for-4.14 1/2] pvcalls: Clearly spell out that the > header is just a reference > > On 27.06.20 11:55, Julien Grall wrote: > > From: Julien Grall > > > > A recent thread on xen-devel [1] pointed out that the header was > > provided as a reference for the specification. > > > > Unfortunately, this was never written down in xen.git so for an external > > user (or a reviewer) it is not clear whether the spec or the header > > shoudl be followed when there is a conflict. > > should > > > > > To avoid more confusion, a paragraph is added at the top of the header > > to clearly spell out it is only provided for reference. > > > > [1] > > https://lore.kernel.org/xen-devel/alpine.DEB.2.21.2006151343430.9074@sstabellini-ThinkPad-T480s/ > > > > Signed-off-by: Julien Grall > > Reviewed-by: Juergen Gross > Release-acked-by: Paul Durrant > > Juergen
Re: [PATCH for-4.14 v3] mm: fix public declaration of struct xen_mem_acquire_resource
On 26.06.2020 17:39, Roger Pau Monne wrote:
> XENMEM_acquire_resource and it's related structure is currently inside
> a __XEN__ or __XEN_TOOLS__ guarded section to limit it's scope to the
> hypervisor or the toolstack only. This is wrong as the hypercall is
> already being used by the Linux kernel at least, and as such needs to
> be public.
>
> Also switch the usage of uint64_aligned_t to plain uint64_t, as
> uint64_aligned_t is only to be used by the toolstack. Doing such
> change will reduce the size of the structure on 32bit x86 by 4bytes,
> since there will be no padding added after the frame_list handle.
>
> This is fine, as users of the previous layout will allocate 4bytes of
> padding that won't be read by Xen, and users of the new layout won't
> allocate those, which is also fine since Xen won't try to access them.
>
> Note that the structure already has compat handling, and such handling
> will take care of copying the right size (ie: minus the padding) when
> called from a 32bit x86 context. This is true for the compat code both
> before and after this patch, since the structures in the memory.h
> compat header are subject to a pragma pack(4), which already removed
> the trailing padding that would otherwise be introduced by the
> alignment of the frame field to 8 bytes.
>
> Fixes: 3f8f12281dd20 ('x86/mm: add HYPERVISOR_memory_op to acquire guest
> resources')
> Signed-off-by: Roger Pau Monné
Reviewed-by: Jan Beulich
Re: [PATCH 1/2] xen/displif: Protocol version 2
On 20.05.20 11:04, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko 1. Change protocol version from string to integer Version string, which is in fact an integer, is hard to handle in the code that supports different protocol versions. To simplify that make the version an integer. 2. Pass buffer offset with XENDISPL_OP_DBUF_CREATE There are cases when display data buffer is created with non-zero offset to the data start. Handle such cases and provide that offset while creating a display buffer. 3. Add XENDISPL_OP_GET_EDID command Add an optional request for reading Extended Display Identification Data (EDID) structure which allows better configuration of the display connectors over the configuration set in XenStore. With this change connectors may have multiple resolutions defined with respect to detailed timing definitions and additional properties normally provided by displays. If this request is not supported by the backend then visible area is defined by the relevant XenStore's "resolution" property. If backend provides extended display identification data (EDID) with XENDISPL_OP_GET_EDID request then EDID values must take precedence over the resolutions defined in XenStore. 4. Bump protocol version to 2. Signed-off-by: Oleksandr Andrushchenko --- xen/include/public/io/displif.h | 83 +++-- 1 file changed, 80 insertions(+), 3 deletions(-) diff --git a/xen/include/public/io/displif.h b/xen/include/public/io/displif.h index cc5de9cb1f35..4d43ba5078c8 100644 --- a/xen/include/public/io/displif.h +++ b/xen/include/public/io/displif.h @@ -38,7 +38,7 @@ * Protocol version ** */ -#define XENDISPL_PROTOCOL_VERSION "1" +#define XENDISPL_PROTOCOL_VERSION 2 This is not very nice regarding compatibility. Can't you add a new macro for the integer value? And please add comments further down which additions are related to the new version. Juergen
RE: [PATCH] xen: introduce xen_vring_use_dma
> Subject: Re: [PATCH] xen: introduce xen_vring_use_dma > > On Mon, Jun 29, 2020 at 06:25:41AM +, Peng Fan wrote: > > > > > > Anyway, re-reading the last messages of the original thread > > > > > > [1], it looks like Peng had a clear idea on how to fix the general > > > > > > issue. > > > > > > Peng, what happened with that? > > > > > > > > We shrinked the rpmsg reserved area to workaround the issue. > > > > So still use the dma apis in rpmsg. > > > > > > > > But here I am going to address domu android trusty issue using virtio. > > > > > > My suggestion is to first of all fix DMA API so it works properly. > > > > Could you please elaborate more details? > > > > You mean the DMA API usage of rpmsg? Or xen domu dma_ops? > > > > Thanks, > > Peng. > > Not 100% sure but I think xen dma ops. Sorry to ask again, could you explain more details about what issues do you see of current xen ARM domu dma_ops? Or Dom0 swiotlb xen dma_ops? Thanks, Pen.g > > -- > MST
Re: [PATCH] xen: introduce xen_vring_use_dma
On Mon, Jun 29, 2020 at 06:25:41AM +, Peng Fan wrote: > > > > > Anyway, re-reading the last messages of the original thread [1], > > > > > it looks like Peng had a clear idea on how to fix the general issue. > > > > > Peng, what happened with that? > > > > > > We shrinked the rpmsg reserved area to workaround the issue. > > > So still use the dma apis in rpmsg. > > > > > > But here I am going to address domu android trusty issue using virtio. > > > > My suggestion is to first of all fix DMA API so it works properly. > > Could you please elaborate more details? > > You mean the DMA API usage of rpmsg? Or xen domu dma_ops? > > Thanks, > Peng. Not 100% sure but I think xen dma ops. -- MST
RE: [PATCH] xen: introduce xen_vring_use_dma
> Subject: Re: [PATCH] xen: introduce xen_vring_use_dma > > On Mon, Jun 29, 2020 at 03:05:19AM +, Peng Fan wrote: > > > Subject: Re: [PATCH] xen: introduce xen_vring_use_dma > > > > > > On Thu, Jun 25, 2020 at 10:31:27AM -0700, Stefano Stabellini wrote: > > > > On Wed, 24 Jun 2020, Michael S. Tsirkin wrote: > > > > > On Wed, Jun 24, 2020 at 02:53:54PM -0700, Stefano Stabellini wrote: > > > > > > On Wed, 24 Jun 2020, Michael S. Tsirkin wrote: > > > > > > > On Wed, Jun 24, 2020 at 10:59:47AM -0700, Stefano Stabellini > wrote: > > > > > > > > On Wed, 24 Jun 2020, Michael S. Tsirkin wrote: > > > > > > > > > On Wed, Jun 24, 2020 at 05:17:32PM +0800, Peng Fan wrote: > > > > > > > > > > Export xen_swiotlb for all platforms using xen swiotlb > > > > > > > > > > > > > > > > > > > > Use xen_swiotlb to determine when vring should use dma > > > > > > > > > > APIs to map the > > > > > > > > > > ring: when xen_swiotlb is enabled the dma API is required. > > > > > > > > > > When it is disabled, it is not required. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Peng Fan > > > > > > > > > > > > > > > > > > Isn't there some way to use VIRTIO_F_IOMMU_PLATFORM for > > > this? > > > > > > > > > Xen was there first, but everyone else is using that now. > > > > > > > > > > > > > > > > Unfortunately it is complicated and it is not related to > > > > > > > > VIRTIO_F_IOMMU_PLATFORM :-( > > > > > > > > > > > > > > > > > > > > > > > > The Xen subsystem in Linux uses dma_ops via swiotlb_xen to > > > > > > > > translate foreign mappings (memory coming from other VMs) > > > > > > > > to > > > physical addresses. > > > > > > > > On x86, it also uses dma_ops to translate Linux's idea of > > > > > > > > a physical address into a real physical address (this is > > > > > > > > unneeded on ARM.) > > > > > > > > > > > > > > > > > > > > > > > > So regardless of VIRTIO_F_IOMMU_PLATFORM, dma_ops should > > > > > > > > be used on Xen/x86 always and on Xen/ARM if Linux is Dom0 > > > > > > > > (because it has foreign > > > > > > > > mappings.) That is why we have the if (xen_domain) return > > > > > > > > true; in vring_use_dma_api. > > > > > > > > > > > > > > VIRTIO_F_IOMMU_PLATFORM makes guest always use DMA ops. > > > > > > > > > > > > > > Xen hack predates VIRTIO_F_IOMMU_PLATFORM so it *also* > > > > > > > forces DMA ops even if VIRTIO_F_IOMMU_PLATFORM is clear. > > > > > > > > > > > > > > Unfortunately as a result Xen never got around to properly > > > > > > > setting VIRTIO_F_IOMMU_PLATFORM. > > > > > > > > > > > > I don't think VIRTIO_F_IOMMU_PLATFORM would be correct for > > > > > > this because the usage of swiotlb_xen is not a property of > > > > > > virtio, > > > > > > > > > > > > > > > Basically any device without VIRTIO_F_ACCESS_PLATFORM (that is > > > > > it's name in latest virtio spec, VIRTIO_F_IOMMU_PLATFORM is what > > > > > linux calls it) is declared as "special, don't follow normal > > > > > rules for access". > > > > > > > > > > So yes swiotlb_xen is not a property of virtio, but what *is* a > > > > > property of virtio is that it's not special, just a regular > > > > > device from DMA > > > POV. > > > > > > > > I am trying to understand what you meant but I think I am missing > > > > something. > > > > > > > > Are you saying that modern virtio should always have > > > > VIRTIO_F_ACCESS_PLATFORM, hence use normal dma_ops as any other > > > devices? > > > > > > I am saying it's a safe default. Clear VIRTIO_F_ACCESS_PLATFORM if > > > you have some special needs e.g. you are very sure it's ok to bypass > > > DMA ops, or you need to support a legacy guest (produced in the > > > window between virtio 1 support in 2014 and support for > VIRTIO_F_ACCESS_PLATFORM in 2016). > > > > > > > > > > If that is the case, how is it possible that virtio breaks on ARM > > > > using the default dma_ops? The breakage is not Xen related (except > > > > that Xen turns dma_ops on). The original message from Peng was: > > > > > > > > vring_map_one_sg -> vring_use_dma_api > > > >-> dma_map_page > > > >-> __swiotlb_map_page > > > > ->swiotlb_map_page > > > > > > > > ->__dma_map_area(phys_to_virt(dma_to_phys(dev, > > > dev_addr)), size, dir); > > > > However we are using per device dma area for rpmsg, phys_to_virt > > > > could not return a correct virtual address for virtual address in > > > > vmalloc area. Then kernel panic. > > > > > > > > I must be missing something. Maybe it is because it has to do with > RPMesg? > > > > > > I think it's an RPMesg bug, yes > > > > rpmsg bug is another issue, it should not use dma_alloc_coherent for > > reserved area, and use vmalloc_to_page. > > > > Anyway here using dma api will also trigger issue. > > > > > > > > > > > > > > > > > You might have noticed that I missed one possible case above: > > > > > > > > Xen/ARM DomU :-) > > > > > > > > > > > > > > > > Xen/ARM domUs don't need
Re: [PATCH] xen: introduce xen_vring_use_dma
On Mon, Jun 29, 2020 at 03:05:19AM +, Peng Fan wrote: > > Subject: Re: [PATCH] xen: introduce xen_vring_use_dma > > > > On Thu, Jun 25, 2020 at 10:31:27AM -0700, Stefano Stabellini wrote: > > > On Wed, 24 Jun 2020, Michael S. Tsirkin wrote: > > > > On Wed, Jun 24, 2020 at 02:53:54PM -0700, Stefano Stabellini wrote: > > > > > On Wed, 24 Jun 2020, Michael S. Tsirkin wrote: > > > > > > On Wed, Jun 24, 2020 at 10:59:47AM -0700, Stefano Stabellini wrote: > > > > > > > On Wed, 24 Jun 2020, Michael S. Tsirkin wrote: > > > > > > > > On Wed, Jun 24, 2020 at 05:17:32PM +0800, Peng Fan wrote: > > > > > > > > > Export xen_swiotlb for all platforms using xen swiotlb > > > > > > > > > > > > > > > > > > Use xen_swiotlb to determine when vring should use dma > > > > > > > > > APIs to map the > > > > > > > > > ring: when xen_swiotlb is enabled the dma API is required. > > > > > > > > > When it is disabled, it is not required. > > > > > > > > > > > > > > > > > > Signed-off-by: Peng Fan > > > > > > > > > > > > > > > > Isn't there some way to use VIRTIO_F_IOMMU_PLATFORM for > > this? > > > > > > > > Xen was there first, but everyone else is using that now. > > > > > > > > > > > > > > Unfortunately it is complicated and it is not related to > > > > > > > VIRTIO_F_IOMMU_PLATFORM :-( > > > > > > > > > > > > > > > > > > > > > The Xen subsystem in Linux uses dma_ops via swiotlb_xen to > > > > > > > translate foreign mappings (memory coming from other VMs) to > > physical addresses. > > > > > > > On x86, it also uses dma_ops to translate Linux's idea of a > > > > > > > physical address into a real physical address (this is > > > > > > > unneeded on ARM.) > > > > > > > > > > > > > > > > > > > > > So regardless of VIRTIO_F_IOMMU_PLATFORM, dma_ops should be > > > > > > > used on Xen/x86 always and on Xen/ARM if Linux is Dom0 > > > > > > > (because it has foreign > > > > > > > mappings.) That is why we have the if (xen_domain) return > > > > > > > true; in vring_use_dma_api. > > > > > > > > > > > > VIRTIO_F_IOMMU_PLATFORM makes guest always use DMA ops. > > > > > > > > > > > > Xen hack predates VIRTIO_F_IOMMU_PLATFORM so it *also* forces > > > > > > DMA ops even if VIRTIO_F_IOMMU_PLATFORM is clear. > > > > > > > > > > > > Unfortunately as a result Xen never got around to properly > > > > > > setting VIRTIO_F_IOMMU_PLATFORM. > > > > > > > > > > I don't think VIRTIO_F_IOMMU_PLATFORM would be correct for this > > > > > because the usage of swiotlb_xen is not a property of virtio, > > > > > > > > > > > > Basically any device without VIRTIO_F_ACCESS_PLATFORM (that is it's > > > > name in latest virtio spec, VIRTIO_F_IOMMU_PLATFORM is what linux > > > > calls it) is declared as "special, don't follow normal rules for > > > > access". > > > > > > > > So yes swiotlb_xen is not a property of virtio, but what *is* a > > > > property of virtio is that it's not special, just a regular device from > > > > DMA > > POV. > > > > > > I am trying to understand what you meant but I think I am missing > > > something. > > > > > > Are you saying that modern virtio should always have > > > VIRTIO_F_ACCESS_PLATFORM, hence use normal dma_ops as any other > > devices? > > > > I am saying it's a safe default. Clear VIRTIO_F_ACCESS_PLATFORM if you have > > some special needs e.g. you are very sure it's ok to bypass DMA ops, or you > > need to support a legacy guest (produced in the window between virtio 1 > > support in 2014 and support for VIRTIO_F_ACCESS_PLATFORM in 2016). > > > > > > > If that is the case, how is it possible that virtio breaks on ARM > > > using the default dma_ops? The breakage is not Xen related (except > > > that Xen turns dma_ops on). The original message from Peng was: > > > > > > vring_map_one_sg -> vring_use_dma_api > > >-> dma_map_page > > > -> __swiotlb_map_page > > > ->swiotlb_map_page > > > > > > ->__dma_map_area(phys_to_virt(dma_to_phys(dev, > > dev_addr)), size, dir); > > > However we are using per device dma area for rpmsg, phys_to_virt > > > could not return a correct virtual address for virtual address in > > > vmalloc area. Then kernel panic. > > > > > > I must be missing something. Maybe it is because it has to do with RPMesg? > > > > I think it's an RPMesg bug, yes > > rpmsg bug is another issue, it should not use dma_alloc_coherent for reserved > area, > and use vmalloc_to_page. > > Anyway here using dma api will also trigger issue. > > > > > > > > > > > > > You might have noticed that I missed one possible case above: > > > > > > > Xen/ARM DomU :-) > > > > > > > > > > > > > > Xen/ARM domUs don't need swiotlb_xen, it is not even > > > > > > > initialized. So if > > > > > > > (xen_domain) return true; would give the wrong answer in that > > > > > > > case. > > > > > > > Linux would end up calling the "normal" dma_ops, not > > > > > > > swiotlb-xen, and the "normal" dma_ops fail. > > > >
