[xen-unstable test] 164055: regressions - FAIL
flight 164055 xen-unstable real [real] flight 164059 xen-unstable real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/164055/ http://logs.test-lab.xenproject.org/osstest/logs/164059/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 12 debian-hvm-install fail REGR. vs. 164049 Tests which are failing intermittently (not blocking): test-amd64-amd64-examine 4 memdisk-try-append fail pass in 164059-retest Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 164049 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 164049 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 164049 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 164049 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 164049 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 164049 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 164049 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 164049 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 164049 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 164049 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 164049 test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass version targeted for testing: xen 3747a2bb67daa5a8baeff6cda57dc98a5ef79c3e baseline version: xen 58ad654ebce7ccb272a3f4f3482c03aaad850d31 Last test of basis 164049 2021-07-30 01:52:50 Z1 days Testing same since 164055 2021-07-30 14:37:59 Z0 days1 attempt
[linux-linus test] 164056: regressions - FAIL
flight 164056 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/164056/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemut-rhel6hvm-intel 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-ws16-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-debianhvm-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-examine 6 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-libvirt 7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemuu-rhel6hvm-amd 7 xen-installfail REGR. vs. 152332 test-amd64-i386-xl-qemut-ws16-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemut-rhel6hvm-amd 7 xen-installfail REGR. vs. 152332 test-amd64-i386-xl7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-debianhvm-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-coresched-i386-xl 7 xen-install fail REGR. vs. 152332 test-amd64-i386-pair 10 xen-install/src_host fail REGR. vs. 152332 test-amd64-i386-pair 11 xen-install/dst_host fail REGR. vs. 152332 test-amd64-i386-libvirt-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-xsm7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemuu-rhel6hvm-intel 7 xen-install fail REGR. vs. 152332 test-amd64-i386-freebsd10-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-pvshim 7 xen-install fail REGR. vs. 152332 test-amd64-i386-freebsd10-i386 7 xen-installfail REGR. vs. 152332 test-amd64-i386-xl-shadow 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-raw7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-ovmf-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-win7-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-win7-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-libvirt-pair 10 xen-install/src_host fail REGR. vs. 152332 test-amd64-i386-libvirt-pair 11 xen-install/dst_host fail REGR. vs. 152332 test-arm64-arm64-xl-thunderx 13 debian-fixup fail REGR. vs. 152332 test-arm64-arm64-xl 14 guest-start fail REGR. vs. 152332 test-amd64-amd64-amd64-pvgrub 20 guest-stop fail REGR. vs. 152332 test-arm64-arm64-xl-credit1 13 debian-fixup fail REGR. vs. 152332 test-amd64-amd64-i386-pvgrub 20 guest-stop fail REGR. vs. 152332 test-arm64-arm64-xl-credit2 13 debian-fixup fail REGR. vs. 152332 test-arm64-arm64-libvirt-xsm 13 debian-fixup fail REGR. vs. 152332 test-arm64-arm64-xl-xsm 14 guest-startfail in 164051 REGR. vs. 152332 Tests which are failing intermittently (not blocking): test-armhf-armhf-xl-arndale 10 host-ping-check-xen fail in 164051 pass in 164056 test-arm64-arm64-xl 13 debian-fixup fail in 164051 pass in 164056 test-arm64-arm64-xl-xsm 13 debian-fixup fail pass in 164051 test-amd64-amd64-examine 4 memdisk-try-append fail pass in 164051 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 152332 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 152332 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 152332 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 152332 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 152332 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 152332 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 152332 test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf
Re: [RFC PATCH] xen/memory: Introduce a hypercall to provide unallocated space
On Fri, 30 Jul 2021, Oleksandr wrote: > Hi Andrew, Julien. > > > @Andrew, I think that arguments you provided in your first answer (why the > proposed solution is a potentially racy and not a safe) are valid and > reasonable. > Thanks for the feedback. > > > On 28.07.21 22:53, Julien Grall wrote: > > > > > > On 28/07/2021 20:00, Andrew Cooper wrote: > > > On 28/07/2021 18:27, Julien Grall wrote: > > > > Hi Andrew, > > > > > > > > On 28/07/2021 18:19, Andrew Cooper wrote: > > > > > On 28/07/2021 17:18, Oleksandr Tyshchenko wrote: > > > > > > From: Oleksandr Tyshchenko > > > > > > > > > > > > Add XENMEM_get_unallocated_space hypercall which purpose is to > > > > > > query hypervisor to find regions of guest physical address space > > > > > > which are unused and can be used to create grant/foreign mappings > > > > > > instead of wasting real pages from the domain memory for > > > > > > establishing these mappings. The problem with the current Linux > > > > > > on Xen on Arm behaviour is if we want to map some guest memory > > > > > > regions in advance or to perform cache mappings in the backend > > > > > > we might run out of memory in the host (see XSA-300). > > > > > > This of course, depends on the both host and guest memory sizes. > > > > > > > > > > > > The "unallocated space" can't be figured out precisely by > > > > > > the domain on Arm without hypervisor involvement: > > > > > > - not all device I/O regions are known by the time domain starts > > > > > > creating grant/foreign mappings > > > > > > - the Dom0 is not aware of memory regions used for the identity > > > > > > mappings needed for the PV drivers to work > > > > > > In both cases we might end up re-using these regions by > > > > > > a mistake. So, the hypervisor which maintains the P2M for the domain > > > > > > is in the best position to provide "unallocated space". > > > > > > > > > > I'm afraid this does not improve the situation. > > > > > > > > > > If a guest follows the advice from XENMEM_get_unallocated_space, and > > > > > subsequently a new IO or identity region appears, everything will > > > > > explode, because the "safe area" wasn't actually safe. > > > > > > > > > > The safe range *must* be chosen by the toolstack, because nothing else > > > > > can do it safely or correctly. > > > > > > > > The problem is how do you size it? In particular, a backend may map > > > > multiple time the same page (for instance if the page is granted twice). > > > > > > The number of mapped grants is limited by the size of the maptrack table > > > in Xen, which is a toolstack input to the domaincreate hypercall. > > > Therefore, the amount of space required is known and bounded. > > > > > > There are a handful of other frames required in the current ABI (shared > > > info, vcpu info, etc). > > > > > > The areas where things do become fuzzy is things like foreign mappings, > > > acquire_resource, etc for the control domain, which are effectively > > > unbounded from the domain's point of view. > > > > > > For those, its entirely fine to say "here 128G of safe mapping space" or > > > so. Even the quantity of mapping dom0 can make is limited by the shadow > > > memory pool and the number of pagetables Xen is willing to expend on the > > > second stage translation tables. > > > > FWIW, on Arm, we don't have shadow memory pool. > > > > Anyway, it should be possible to give a 128GB "safe range" and let Xen deal > > with it. > > > > > > > > > > > > > > > > > > > Once a safe range (or ranges) has been chosen, any subsequent action > > > > > which overlaps with the ranges must be rejected, as it will violate > > > > > the > > > > > guarantees provided. > > > > > > > > > > Furthermore, the ranges should be made available to the guest via > > > > > normal > > > > > memory map means. On x86, this is via the E820 table, and on ARM I > > > > > presume the DTB. There is no need for a new hypercall. > > > > > > > > Device-Tree only works if you have a guest using it. How about ACPI? > > > > > > ACPI inherits E820 from x86 (its a trivial format), and UEFI was also > > > based on it. > > > > > > But whichever... All firmware interfaces have a memory map. > > > > This will be UEFI memory map. However, I am a bit confused how we can tell > > the OS the region will be used for grant/foreign mapping. Is it possible to > > reserved a new type? > > > > > > > > > To me the hypercall solution at least: > > > > 1) Avoid to have to define the region on every single firmware table > > > > > > There is only ever one. > > > > Why? I could forsee an interest to use the host memory map and therefore we > > may need to find a few holes for safe regions to use. > > > > > > > > > 2) Allow to easily extend after the guest run > > > > > > The safe ranges can't be changed (safely). This is the same problem as > > > needing to know things like your PCI apertures ahead of time, or where > > > the DIMM hotplug regions are. > > > > >
[xen-unstable-smoke test] 164057: tolerable all pass - PUSHED
flight 164057 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/164057/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass version targeted for testing: xen e066ca5acc2ee3b5db5c005e1a548b05e753e07d baseline version: xen 604551fb763c4c70123f642a9b2866890790e2b2 Last test of basis 164054 2021-07-30 14:01:37 Z0 days Testing same since 164057 2021-07-30 19:02:47 Z0 days1 attempts People who touched revisions under test: Julien Grall 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 604551fb76..e066ca5acc e066ca5acc2ee3b5db5c005e1a548b05e753e07d -> smoke
Re: [PATCH v5 2/4] xen: do not return -EEXIST if iommu_add_dt_device is called twice
On Mon, 26 Jul 2021, Julien Grall wrote: > Hi Stefano, > > On 23/07/2021 21:16, Stefano Stabellini wrote: > > On Fri, 23 Jul 2021, Julien Grall wrote: > > > > Signed-off-by: Stefano Stabellini > > > > --- > > > > Changes in v5: > > > > - new patch > > > > --- > > > >xen/drivers/passthrough/device_tree.c | 9 +++-- > > > >1 file changed, 7 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/xen/drivers/passthrough/device_tree.c > > > > b/xen/drivers/passthrough/device_tree.c > > > > index 999b831d90..32526ecabb 100644 > > > > --- a/xen/drivers/passthrough/device_tree.c > > > > +++ b/xen/drivers/passthrough/device_tree.c > > > > @@ -140,8 +140,13 @@ int iommu_add_dt_device(struct dt_device_node *np) > > > >if ( !ops ) > > > >return -EINVAL; > > > >+/* > > > > + * Some Device Trees may expose both legacy SMMU and generic > > > > + * IOMMU bindings together. If both are present, the device > > > > + * can be already added. > > > > > > Wouldn't this also happen when there is just generic bindings? If so, > > > shouldn't this patch be first in the series to avoid breaking bisection? > > > > No, both need to be present; if there is just the generic bindings we > > don't need this change. I can still move it to the beginning of the > > series anyway if you prefer. > > Sorry but I am having some trouble to understand why this is not a problem for > just the legacy binding. > > If I look at patch #1, the dev->iommu_fspec will be allocated in > register_smmu_master(). If I am not mistaken, this is called when the SMMU is > initialized. > > So the call to iommu_add_dt_device() in handle_device() should return -EEXIST > (dev_iommu_fwspec_get() will return a non-NULL pointer). > > What did I miss? I checked. It is true that we need this check with the legacy bindings, even when alone. Like you said, dev->iommu_fspec is allocated early by register_smmu_master. Then, handle_device, or handle_passthrough_prop for dom0less guests, calls iommu_add_dt_device a second time. On the other hand with only the generic bindings register_smmu_master doesn't call iommu_add_dt_device, so iommu_add_dt_device is called only once by handle_device or handle_passthrough_prop. The comment I proposed is not correct. What about this one? /* * In case of legacy SMMU bindings, register_smmu_master might have * already initialized struct iommu_fwspec for this device. */
[qemu-mainline test] 164052: tolerable FAIL - PUSHED
flight 164052 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/164052/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 164046 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 164046 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 164046 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 164046 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 164046 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 164046 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 164046 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass version targeted for testing: qemuu7742fe64e5c2c2c9f9787d107b693eaac602eaae baseline version: qemuu768832575d2e37042d00eb693cda809cb30981d4 Last test of basis 164046 2021-07-29 22:09:08 Z0 days Testing same since 164052 2021-07-30 08:36:47 Z0 days1 attempts People who touched revisions under test: Gerd Hoffmann Peter Maydell jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass buil
Re: [PATCH v1 4/5] PCI: Adapt all code locations to not use struct pci_dev::driver directly
On 7/29/21 4:37 PM, Uwe Kleine-König wrote:
> --- a/drivers/pci/xen-pcifront.c
> +++ b/drivers/pci/xen-pcifront.c
> @@ -599,12 +599,12 @@ static pci_ers_result_t pcifront_common_process(int cmd,
> result = PCI_ERS_RESULT_NONE;
>
> pcidev = pci_get_domain_bus_and_slot(domain, bus, devfn);
> - if (!pcidev || !pcidev->driver) {
> + pdrv = pci_driver_of_dev(pcidev);
> + if (!pcidev || !pdrv) {
If pcidev is NULL we are dead by the time we reach 'if' statement.
-boris
> dev_err(&pdev->xdev->dev, "device or AER driver is NULL\n");
> pci_dev_put(pcidev);
> return result;
> }
> - pdrv = pcidev->driver;
>
[PATCH v2] tools/xl: Add device_model_stubdomain_init_seclabel option to xl.cfg
This adds an option to the xl domain configuration syntax for specifying
a build-time XSM security label for device-model stubdomains separate
from the run-time label specified by 'device_model_stubdomain_seclabel'.
Fields are also added to the 'libxl_domain_build_info' struct to contain
the new information, and a new call to 'xc_flask_relabel_domain'
inserted to affect the change at the appropriate time.
The device-model stubdomain is relabeled at the same time as its guest,
just before the guest is unpaused. This requires the stubdomain itself
to be unpaused and run for a short time prior to being relabeled, but
allows PCI device assignments specified in xl.cfg to be completed prior
to relabeling. This choice allows the privileges required to perform
assignments to be dropped in the relabeling.
The implementation mirrors that of the 'seclabel' and 'init_seclabel'
options for user domains. When all used in concert, this enables the
creation of security policies that minimize run-time privileges between
the toolstack domain, device-model stubdomains, and user domains.
Signed-off-by: Scott Davis
---
Changes in v2:
- removed superfluous chanegs to libxl_dm.c
- changed all security label lookup failures due to FLASK being disabled
from warnings to hard errors based on mailing list discussion
- added discussion of relabel point to commit message
---
docs/man/xl.cfg.5.pod.in | 10 +++
tools/golang/xenlight/helpers.gen.go | 5
tools/golang/xenlight/types.gen.go | 2 ++
tools/include/libxl.h| 10 +++
tools/libs/light/libxl_create.c | 42
tools/libs/light/libxl_types.idl | 2 ++
tools/xl/xl_parse.c | 12 +++-
7 files changed, 71 insertions(+), 12 deletions(-)
diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index 4b1e3028d2..7c8a696d61 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -2766,6 +2766,16 @@ you have selected.
Assign an XSM security label to the device-model stubdomain.
+=item B
+
+Specify a temporary XSM security label for the device-model stubdomain used
+during creation of it and its associated guest. The stubdomain's XSM label will
+then be changed to the execution seclabel (as specified by
+B) once creation is complete, prior to
+unpausing the stubdomain's guest. With proper (re)labeling, a security policy
+can be constructed that minimizes run-time privileges between the toolstack
+domain, device-model stubdomains, and user domains.
+
=item B
Pass additional arbitrary options on the device-model command
diff --git a/tools/golang/xenlight/helpers.gen.go
b/tools/golang/xenlight/helpers.gen.go
index bfc1e7f312..a70eb5eb58 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -1023,6 +1023,8 @@ x.StubdomainRamdisk = C.GoString(xc.stubdomain_ramdisk)
x.DeviceModel = C.GoString(xc.device_model)
x.DeviceModelSsidref = uint32(xc.device_model_ssidref)
x.DeviceModelSsidLabel = C.GoString(xc.device_model_ssid_label)
+x.DeviceModelExecSsidref = uint32(xc.device_model_exec_ssidref)
+x.DeviceModelExecSsidLabel = C.GoString(xc.device_model_exec_ssid_label)
x.DeviceModelUser = C.GoString(xc.device_model_user)
if err := x.Extra.fromC(&xc.extra);err != nil {
return fmt.Errorf("converting field Extra: %v", err)
@@ -1354,6 +1356,9 @@ xc.device_model = C.CString(x.DeviceModel)}
xc.device_model_ssidref = C.uint32_t(x.DeviceModelSsidref)
if x.DeviceModelSsidLabel != "" {
xc.device_model_ssid_label = C.CString(x.DeviceModelSsidLabel)}
+xc.device_model_exec_ssidref = C.uint32_t(x.DeviceModelExecSsidref)
+if x.DeviceModelExecSsidLabel != "" {
+xc.device_model_exec_ssid_label = C.CString(x.DeviceModelExecSsidLabel)}
if x.DeviceModelUser != "" {
xc.device_model_user = C.CString(x.DeviceModelUser)}
if err := x.Extra.toC(&xc.extra); err != nil {
diff --git a/tools/golang/xenlight/types.gen.go
b/tools/golang/xenlight/types.gen.go
index 09a3bb67e2..a76570cae7 100644
--- a/tools/golang/xenlight/types.gen.go
+++ b/tools/golang/xenlight/types.gen.go
@@ -488,6 +488,8 @@ StubdomainRamdisk string
DeviceModel string
DeviceModelSsidref uint32
DeviceModelSsidLabel string
+DeviceModelExecSsidref uint32
+DeviceModelExecSsidLabel string
DeviceModelUser string
Extra StringList
ExtraPv StringList
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index b9ba16d698..ca3ec3e53d 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -1081,6 +1081,16 @@ typedef struct libxl__ctx libxl_ctx;
*/
#define LIBXL_HAVE_SSID_LABEL 1
+/*
+ * LIBXL_HAVE_BUILDINFO_DEVICE_MODEL_STUBDOMAIN_EXEC_SSID
+ *
+ * If this is defined, then the libxl_domain_build_info structure will
+ * contain 'device_model_exec_ssidref' and 'device_model_exec_ssid_label' for
+ * specifying a run-time XSM security label separate from the build-time label
+ * specified in 'device_model_ssidref' and 'device_model_ssid_label'.
+ */
+#define LIBX
[xen-unstable-smoke test] 164054: tolerable all pass - PUSHED
flight 164054 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/164054/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass version targeted for testing: xen 604551fb763c4c70123f642a9b2866890790e2b2 baseline version: xen 3747a2bb67daa5a8baeff6cda57dc98a5ef79c3e Last test of basis 164053 2021-07-30 10:02:51 Z0 days Testing same since 164054 2021-07-30 14:01:37 Z0 days1 attempts People who touched revisions under test: Julien Grall 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 3747a2bb67..604551fb76 604551fb763c4c70123f642a9b2866890790e2b2 -> smoke
Re: [PATCH v1 0/5] PCI: Drop duplicated tracking of a pci_dev's bound driver
Hi Andy, On Fri, Jul 30, 2021 at 11:06:20AM +0300, Andy Shevchenko wrote: > On Thu, Jul 29, 2021 at 10:37:35PM +0200, Uwe Kleine-König wrote: > > struct pci_dev tracks the bound pci driver twice. This series is about > > removing this duplication. > > > > The first two patches are just cleanups. The third patch introduces a > > wrapper that abstracts access to struct pci_dev->driver. In the next > > patch (hopefully) all users are converted to use the new wrapper and > > finally the fifth patch removes the duplication. > > > > Note this series is only build tested (allmodconfig on several > > architectures). > > > > I'm open to restructure this series if this simplifies things. E.g. the > > use of the new wrapper in drivers/pci could be squashed into the patch > > introducing the wrapper. Patch 4 could be split by maintainer tree or > > squashed into patch 3 completely. > > I see only patch 4 and this cover letter... The full series is available at https://lore.kernel.org/linux-pci/20210729203740.1377045-1-u.kleine-koe...@pengutronix.de/ All patches but #4 only touch drivers/pci/ (and include/linux/pci.h) and it seemed excessive to me to send all patches to all people. It seems at least for you I balanced this wrongly. The short version is that patch #3 introduces +#define pci_driver_of_dev(pdev) ((pdev)->driver) which allows to do the stuff done in patch #4 and then patch #5 does -#define pci_driver_of_dev(pdev) ((pdev)->driver) +#define pci_driver_of_dev(pdev) ((pdev)->dev.driver ? to_pci_driver((pdev)->dev.driver) : NULL) plus some cleanups. If you want I can send you a bounce (or you try b4 am 20210729203740.1377045-1-u.kleine-koe...@pengutronix.de ). Best regards and thanks for caring, Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
Re: [PATCH v3 2/2] tools/xenstore: set open file descriptor limit for xenstored
Hi Ian,
On 30/07/2021 14:35, Ian Jackson wrote:
Juergen Gross writes ("[PATCH v3 2/2] tools/xenstore: set open file descriptor limit
for xenstored"):
Add a configuration item for the maximum number of domains xenstored
should support and set the limit of open file descriptors accordingly.
For HVM domains there are up to 5 socket connections per domain (2 by
the xl daemon process, and 3 by qemu). So set the ulimit for xenstored
to 5 * XENSTORED_MAX_DOMAINS + 100 (the "+ 100" is for some headroom,
like logging, event channel device, etc.).
...
+## Type: integer
+## Default: 32768
+#
+# Select maximum number of domains supported by xenstored.
+# Only evaluated if XENSTORETYPE is "daemon".
+#XENSTORED_MAX_N_DOMAINS=32768
I approve of doing something about the fd limit. I have some qualms
about the documentation.
The documentation doesn't say what happens if this limit is exceeded.
Also the default of 32758 suggests that we actually support that many
domains. I don't think we do...
I didn't find anything in SUPPORT.md about how many guests we support
but I wouldn't want this setting here to imply full support for 32768
domains.
If you don't want to tackle this can of works, maybe add this:
# This just controls some resource limits for xenstored; if the
# limit is exceeded, xenstored will stop being able to function
# properly for additional guests. The default value is so large
# that it won't be exceeded in a supported configuration, but
# should not be taken to mean that the whole Xen system is
# guaranteed to work properly with that many guests.
Julien, did you ask for this to be made configurable ? Having written
the text above, I wonder if it wouldn't just be better to
unconditionally set it to "unlimited" rather than offering footgun
dressed up like a tuneable...
So in v1 (see [1]), Juergen wanted to raise the limit. I assumed this
meant that the default limit (configured by the system may not be enough).
I felt this was wrong to impose an higher limit on everyone when an
admin may know the maximum number of domains.
By "unlimited", do you mean the calling "ulimit" (or whatever is used
for configuring FDs) with unlimited?
If so, I would be OK with that. My main was was to move the raising the
limit outside Xenstored because:
1) This is easier for an admin to tweak it (in particular the OOM)
2) It feels wrong to me that the daemon chose the limits
3) An admin can enforce it
Cheers,
[1] 1e38cce0-6960-ac21-b349-dac8551e2...@xen.org
--
Julien Grall
Re: [RFC PATCH] xen/memory: Introduce a hypercall to provide unallocated space
Hi Andrew, Julien. @Andrew, I think that arguments you provided in your first answer (why the proposed solution is a potentially racy and not a safe) are valid and reasonable. Thanks for the feedback. On 28.07.21 22:53, Julien Grall wrote: On 28/07/2021 20:00, Andrew Cooper wrote: On 28/07/2021 18:27, Julien Grall wrote: Hi Andrew, On 28/07/2021 18:19, Andrew Cooper wrote: On 28/07/2021 17:18, Oleksandr Tyshchenko wrote: From: Oleksandr Tyshchenko Add XENMEM_get_unallocated_space hypercall which purpose is to query hypervisor to find regions of guest physical address space which are unused and can be used to create grant/foreign mappings instead of wasting real pages from the domain memory for establishing these mappings. The problem with the current Linux on Xen on Arm behaviour is if we want to map some guest memory regions in advance or to perform cache mappings in the backend we might run out of memory in the host (see XSA-300). This of course, depends on the both host and guest memory sizes. The "unallocated space" can't be figured out precisely by the domain on Arm without hypervisor involvement: - not all device I/O regions are known by the time domain starts creating grant/foreign mappings - the Dom0 is not aware of memory regions used for the identity mappings needed for the PV drivers to work In both cases we might end up re-using these regions by a mistake. So, the hypervisor which maintains the P2M for the domain is in the best position to provide "unallocated space". I'm afraid this does not improve the situation. If a guest follows the advice from XENMEM_get_unallocated_space, and subsequently a new IO or identity region appears, everything will explode, because the "safe area" wasn't actually safe. The safe range *must* be chosen by the toolstack, because nothing else can do it safely or correctly. The problem is how do you size it? In particular, a backend may map multiple time the same page (for instance if the page is granted twice). The number of mapped grants is limited by the size of the maptrack table in Xen, which is a toolstack input to the domaincreate hypercall. Therefore, the amount of space required is known and bounded. There are a handful of other frames required in the current ABI (shared info, vcpu info, etc). The areas where things do become fuzzy is things like foreign mappings, acquire_resource, etc for the control domain, which are effectively unbounded from the domain's point of view. For those, its entirely fine to say "here 128G of safe mapping space" or so. Even the quantity of mapping dom0 can make is limited by the shadow memory pool and the number of pagetables Xen is willing to expend on the second stage translation tables. FWIW, on Arm, we don't have shadow memory pool. Anyway, it should be possible to give a 128GB "safe range" and let Xen deal with it. Once a safe range (or ranges) has been chosen, any subsequent action which overlaps with the ranges must be rejected, as it will violate the guarantees provided. Furthermore, the ranges should be made available to the guest via normal memory map means. On x86, this is via the E820 table, and on ARM I presume the DTB. There is no need for a new hypercall. Device-Tree only works if you have a guest using it. How about ACPI? ACPI inherits E820 from x86 (its a trivial format), and UEFI was also based on it. But whichever... All firmware interfaces have a memory map. This will be UEFI memory map. However, I am a bit confused how we can tell the OS the region will be used for grant/foreign mapping. Is it possible to reserved a new type? To me the hypercall solution at least: 1) Avoid to have to define the region on every single firmware table There is only ever one. Why? I could forsee an interest to use the host memory map and therefore we may need to find a few holes for safe regions to use. 2) Allow to easily extend after the guest run The safe ranges can't be changed (safely). This is the same problem as needing to know things like your PCI apertures ahead of time, or where the DIMM hotplug regions are. Having the guest physmap be actually dynamic is the cause of so many bugs (inc security) and misfeatures in Xen. Guests cannot and do no cope with things being fully dynamic, because that's not how real hardware works. What you get is layers and layers of breakage on top of each other, rather than a working system. I would not call it "fully dynamic". Xen can easily know whether a region has ever be allocated before. So long the region has never been allocated, then it should be fine. In fact... The size of mapping space is a limit, just like maxphysaddr, or the PCI apertures, or MMCFG space, etc. You can make it large by default (as it doesn't consume resource when not being used), but any guest OS isn't going to tolerate it morphing during runtime. ... I believe the OS may be not aware of the hotplug region u
[linux-linus test] 164051: regressions - FAIL
flight 164051 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/164051/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemut-rhel6hvm-intel 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-ws16-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-debianhvm-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-examine 6 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-libvirt 7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemuu-rhel6hvm-amd 7 xen-installfail REGR. vs. 152332 test-amd64-i386-xl-qemut-ws16-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemut-rhel6hvm-amd 7 xen-installfail REGR. vs. 152332 test-amd64-i386-xl7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-debianhvm-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-coresched-i386-xl 7 xen-install fail REGR. vs. 152332 test-amd64-i386-pair 10 xen-install/src_host fail REGR. vs. 152332 test-amd64-i386-pair 11 xen-install/dst_host fail REGR. vs. 152332 test-amd64-i386-libvirt-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-xsm7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemuu-rhel6hvm-intel 7 xen-install fail REGR. vs. 152332 test-amd64-i386-freebsd10-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-pvshim 7 xen-install fail REGR. vs. 152332 test-amd64-i386-freebsd10-i386 7 xen-installfail REGR. vs. 152332 test-amd64-i386-xl-shadow 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-raw7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-ovmf-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-win7-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-win7-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-libvirt-pair 10 xen-install/src_host fail REGR. vs. 152332 test-amd64-i386-libvirt-pair 11 xen-install/dst_host fail REGR. vs. 152332 test-armhf-armhf-xl-arndale 10 host-ping-check-xen fail REGR. vs. 152332 test-arm64-arm64-xl-thunderx 13 debian-fixup fail REGR. vs. 152332 test-arm64-arm64-xl 13 debian-fixup fail REGR. vs. 152332 test-arm64-arm64-xl-xsm 14 guest-start fail REGR. vs. 152332 test-amd64-amd64-amd64-pvgrub 20 guest-stop fail REGR. vs. 152332 test-arm64-arm64-xl-credit1 13 debian-fixup fail REGR. vs. 152332 test-amd64-amd64-i386-pvgrub 20 guest-stop fail REGR. vs. 152332 test-arm64-arm64-xl-credit2 13 debian-fixup fail REGR. vs. 152332 test-arm64-arm64-libvirt-xsm 13 debian-fixup fail REGR. vs. 152332 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 152332 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 152332 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 152332 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 152332 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 152332 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 152332 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 152332 test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail n
Re: [PATCH] tools/xenstored: Don't assume errno will not be overwritten in lu_arch()
Hi Juergen, On 30/07/2021 09:40, Juergen Gross wrote: On 29.07.21 17:23, Julien Grall wrote: On 29/07/2021 12:06, Julien Grall wrote: From: Julien Grall At the moment, do_control_lu() will set errno to 0 before calling lu_arch() and then check errno. The expectation is nothing in lu_arch() will change the value unless there is an error. However, per errno(3), a function that succeeds is allowed to change errno. In fact, syslog() will overwrite errno if the logs are rotated at the time it is called. To prevent any further issue, errno is now always set before returning NULL. Additionally, errno is only checked when returning NULL so the client can see the error message if there is any. Reported-by: Michael Kurth Signed-off-by: Julien Grall --- tools/xenstore/xenstored_control.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c index 6b68b79faac7..6fcb42095b59 100644 --- a/tools/xenstore/xenstored_control.c +++ b/tools/xenstore/xenstored_control.c @@ -324,6 +324,7 @@ static const char *lu_binary_alloc(const void *ctx, struct connection *conn, lu_status->kernel_size = size; lu_status->kernel_off = 0; + errno = 0; return NULL; } @@ -339,6 +340,7 @@ static const char *lu_binary_save(const void *ctx, struct connection *conn, memcpy(lu_status->kernel + lu_status->kernel_off, data, size); lu_status->kernel_off += size; + errno = 0; return NULL; } I forgot to update lu_binary(). I will respin the patch once I get some feedback. With setting errno to 0 before returning NULL in lu_binary() you can add Reviewed-by: Juergen Gross Thanks! I will commit it. Cheers, -- Julien Grall
[xen-unstable test] 164049: tolerable FAIL
flight 164049 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/164049/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 164029 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 164029 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 164029 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 164029 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 164029 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 164029 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 164029 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 164029 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 164029 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 164029 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 164029 test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass version targeted for testing: xen 58ad654ebce7ccb272a3f4f3482c03aaad850d31 baseline version: xen 58ad654ebce7ccb272a3f4f3482c03aaad850d31 Last test of basis 164049 2021-07-30 01:52:50 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
[PATCH v3 2/2] tools/xenstore: set open file descriptor limit for xenstored
Juergen Gross writes ("[PATCH v3 2/2] tools/xenstore: set open file descriptor
limit for xenstored"):
> Add a configuration item for the maximum number of domains xenstored
> should support and set the limit of open file descriptors accordingly.
>
> For HVM domains there are up to 5 socket connections per domain (2 by
> the xl daemon process, and 3 by qemu). So set the ulimit for xenstored
> to 5 * XENSTORED_MAX_DOMAINS + 100 (the "+ 100" is for some headroom,
> like logging, event channel device, etc.).
...
> +## Type: integer
> +## Default: 32768
> +#
> +# Select maximum number of domains supported by xenstored.
> +# Only evaluated if XENSTORETYPE is "daemon".
> +#XENSTORED_MAX_N_DOMAINS=32768
I approve of doing something about the fd limit. I have some qualms
about the documentation.
The documentation doesn't say what happens if this limit is exceeded.
Also the default of 32758 suggests that we actually support that many
domains. I don't think we do...
I didn't find anything in SUPPORT.md about how many guests we support
but I wouldn't want this setting here to imply full support for 32768
domains.
If you don't want to tackle this can of works, maybe add this:
# This just controls some resource limits for xenstored; if the
# limit is exceeded, xenstored will stop being able to function
# properly for additional guests. The default value is so large
# that it won't be exceeded in a supported configuration, but
# should not be taken to mean that the whole Xen system is
# guaranteed to work properly with that many guests.
Julien, did you ask for this to be made configurable ? Having written
the text above, I wonder if it wouldn't just be better to
unconditionally set it to "unlimited" rather than offering footgun
dressed up like a tuneable...
If xenstored does go mad or leadk lots of fds, things are basically
stuffed in anycase. Having its syscalls fail with EMFILE is not
really going to help.
Ian.
[xen-unstable-smoke test] 164053: tolerable all pass - PUSHED
flight 164053 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/164053/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass version targeted for testing: xen 3747a2bb67daa5a8baeff6cda57dc98a5ef79c3e baseline version: xen 58ad654ebce7ccb272a3f4f3482c03aaad850d31 Last test of basis 164012 2021-07-27 19:02:43 Z2 days Testing same since 164053 2021-07-30 10:02:51 Z0 days1 attempts People who touched revisions under test: Jane Malalane 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 58ad654ebc..3747a2bb67 3747a2bb67daa5a8baeff6cda57dc98a5ef79c3e -> smoke
[PATCH v3 1/2] tools/xenstore: set oom score for xenstore daemon on Linux
Juergen Gross writes ("[PATCH v3 1/2] tools/xenstore: set oom score for
xenstore daemon on Linux"):
> Xenstored is absolutely mandatory for a Xen host and it can't be
> restarted, so being killed by OOM-killer in case of memory shortage is
> to be avoided.
>
> Set /proc/$pid/oom_score_adj (if available) per default to -500 (this
> translates to 50% of dom0 memory size) in order to allow xenstored to
> use large amounts of memory without being killed.
...
> +## Type: integer
> +## Default: 50
> +#
> +# Percentage of dom0 memory size the xenstore daemon can use before the
> +# OOM killer is allowed to kill it.
> +#XENSTORED_OOM_MEM_THRESHOLD=50
> +
> ## Type: string
> ## Default: @LIBEXEC@/boot/xenstore-stubdom.gz
Thanks for working on this. I approve of the principle.
I have one question about detail:
> }
> + [ -z "$XENSTORED_OOM_MEM_THRESHOLD" ] || XENSTORED_OOM_MEM_THRESHOLD=50
> + XS_OOM_SCORE=-$(($XENSTORED_OOM_MEM_THRESHOLD * 10))
> +
> + rm -f @XEN_RUN_DIR@/xenstored.pid
...
> + XS_PID=`cat @XEN_RUN_DIR@/xenstored.pid`
> + echo $XS_OOM_SCORE >/proc/$XS_PID/oom_score_adj
The effect of all this is that the value specified in
XENSTORED_OOM_MEM_THRESHOLD is transformed before being echoed into
/proc, by being multiplied by -10.
Of course an alternative would be to ask the user to specify the
tuneable directly but given its rather more obscure semantics I think
it is reasonable to have this done by the script.
But maybe we could add something to the doc comment ?
Eg
# (The specified value is multiplied by -10 and echoed into
# /proc/PID/oom_score_adj.)
?
Thanks,
Ian.
Re: [PATCH v2 0/3] xen: remove some checks for always present Xen features
On 7/30/21 3:18 AM, Juergen Gross wrote: > Some features of Xen can be assumed to be always present, so add a > central check to verify this being true and remove the other checks. > > Juergen Gross (3): > xen: check required Xen features > xen: assume XENFEAT_mmu_pt_update_preserve_ad being set for pv guests > xen: assume XENFEAT_gnttab_map_avail_bits being set for pv guests > > arch/x86/xen/enlighten_pv.c | 12 ++-- > arch/x86/xen/mmu_pv.c | 4 ++-- > drivers/xen/features.c | 18 ++ > drivers/xen/gntdev.c| 36 ++-- > 4 files changed, 24 insertions(+), 46 deletions(-) Reviewed-by: Boris Ostrovsky
Re: [PATCH] xen/lib: Fix strcmp() and strncmp()
Andrew Cooper writes ("Re: [PATCH] xen/lib: Fix strcmp() and strncmp()"):
> On 27/07/2021 19:47, Jane Malalane wrote:
> > - register signed char __res;
> > + unsigned char *csu = (unsigned char *)cs;
> > + unsigned char *ctu = (unsigned char *)ct;
>
> So there was actually one final thing, but it is holiday season, hence
> the lack of replies from others.
Oh.
> We should not be casting away const-ness on the pointers, because that
> is undefined behaviour and compilers are starting to warn about it.
I don't think casting away const is UB. Perhaps you (and perhaps
others) are seeing this in 6.3.2.3(2):
| For any qualifier q, a pointer to a non-q-qualified type may be
| converted to a pointer to the q-qualified version of the type; the
| values stored in the original and converted pointers shall compare
| equal.p
This does indeed define the meaning of *adding* qualifiers to a
pointer type but not define the meaning of removing them. But that
whole paragraph is almost redundant, because in 6.3.2.3(7):
| A pointer to an object or incomplete type may be converted to a
| pointer to a different object or incomplete type. If the resulting
| pointer is not correctly aligned57) for the pointed-to type, the
| behavior is undefined. Otherwise, when converted back again, the
| result shall compare equal to the original pointer.
This defines the meaning of conversions of pointers to object types
(like char*) regardless of the qualifiers.
I read that as "a pointer to an object type or to an incomplete type".
But the precise reading doesn't matter because these pointers are
actually to objects.
There's also this in 6.7.3(5):
| If an attempt is made to modify an object defined with a
| const-qualified type through use of an lvalue with
| non-const-qualified type, the behavior is undefined. made to refer
| to an object defined with a volatile-qualified type through
But there is no attempt to modify. (Also this paragraph doesn't apply
because characters in string literals have type char, not type const
char, but 6.4.6(6) directly prohibits modification of characters in
string literals.)
6.2.7(2) says
| All declarations that refer to the same object or function shall
| have compatible type; otherwise, the behavior is undefined.
but I don't think these pointers variables are declarations of the
chars pointed to.
> Therefore, we want something like:
>
> const unsigned char *csu = (const unsigned char *)cs;
Having said all thst, I agree that that not casting away const would
be better (especially if it generates compiler warnings).
I pushed it already. If thios is UB we should revert it but as I say
I think it isn't, so we can wait for a followup.
Thanks,
Ian.
[PATCH v3 1/2] tools/xenstore: set oom score for xenstore daemon on Linux
Xenstored is absolutely mandatory for a Xen host and it can't be restarted, so being killed by OOM-killer in case of memory shortage is to be avoided. Set /proc/$pid/oom_score_adj (if available) per default to -500 (this translates to 50% of dom0 memory size) in order to allow xenstored to use large amounts of memory without being killed. The percentage of dom0 memory above which the oom killer is allowed to kill xenstored can be set via XENSTORED_OOM_MEM_THRESHOLD in xencommons. Make sure the pid file isn't a left-over from a previous run delete it before starting xenstored. Signed-off-by: Juergen Gross --- V2: - set oom score from launch script (Julien Grall) - split off open file descriptor limit setting (Julien Grall) V3: - make oom killer threshold configurable (Julien Grall) --- tools/hotplug/Linux/init.d/sysconfig.xencommons.in | 7 +++ tools/hotplug/Linux/launch-xenstore.in | 6 ++ 2 files changed, 13 insertions(+) diff --git a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in index 00cf7f91d4..5ad4fe0818 100644 --- a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in +++ b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in @@ -48,6 +48,13 @@ XENSTORED_ARGS= # Only evaluated if XENSTORETYPE is "daemon". #XENSTORED_TRACE=[yes|on|1] +## Type: integer +## Default: 50 +# +# Percentage of dom0 memory size the xenstore daemon can use before the +# OOM killer is allowed to kill it. +#XENSTORED_OOM_MEM_THRESHOLD=50 + ## Type: string ## Default: @LIBEXEC@/boot/xenstore-stubdom.gz # diff --git a/tools/hotplug/Linux/launch-xenstore.in b/tools/hotplug/Linux/launch-xenstore.in index 019f9d6f4d..1747c96065 100644 --- a/tools/hotplug/Linux/launch-xenstore.in +++ b/tools/hotplug/Linux/launch-xenstore.in @@ -59,11 +59,17 @@ test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . @CONFIG_DIR@/@CONFIG_LEAF echo "No xenstored found" exit 1 } + [ -z "$XENSTORED_OOM_MEM_THRESHOLD" ] || XENSTORED_OOM_MEM_THRESHOLD=50 + XS_OOM_SCORE=-$(($XENSTORED_OOM_MEM_THRESHOLD * 10)) + + rm -f @XEN_RUN_DIR@/xenstored.pid echo -n Starting $XENSTORED... $XENSTORED --pid-file @XEN_RUN_DIR@/xenstored.pid $XENSTORED_ARGS systemd-notify --booted 2>/dev/null || timeout_xenstore $XENSTORED || exit 1 + XS_PID=`cat @XEN_RUN_DIR@/xenstored.pid` + echo $XS_OOM_SCORE >/proc/$XS_PID/oom_score_adj exit 0 } -- 2.26.2
[PATCH v3 2/2] tools/xenstore: set open file descriptor limit for xenstored
Add a configuration item for the maximum number of domains xenstored
should support and set the limit of open file descriptors accordingly.
For HVM domains there are up to 5 socket connections per domain (2 by
the xl daemon process, and 3 by qemu). So set the ulimit for xenstored
to 5 * XENSTORED_MAX_DOMAINS + 100 (the "+ 100" is for some headroom,
like logging, event channel device, etc.).
Signed-off-by: Juergen Gross
---
V2:
- set ulimit form launch script (Julien Grall)
- split off from original patch (Julien Grall)
---
tools/hotplug/Linux/init.d/sysconfig.xencommons.in | 7 +++
tools/hotplug/Linux/launch-xenstore.in | 3 +++
2 files changed, 10 insertions(+)
diff --git a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
index 5ad4fe0818..2b682415f4 100644
--- a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
+++ b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
@@ -32,6 +32,13 @@
# Changing this requires a reboot to take effect.
#XENSTORED=@XENSTORED@
+## Type: integer
+## Default: 32768
+#
+# Select maximum number of domains supported by xenstored.
+# Only evaluated if XENSTORETYPE is "daemon".
+#XENSTORED_MAX_N_DOMAINS=32768
+
## Type: string
## Default: ""
#
diff --git a/tools/hotplug/Linux/launch-xenstore.in
b/tools/hotplug/Linux/launch-xenstore.in
index 1747c96065..3f8b33dd32 100644
--- a/tools/hotplug/Linux/launch-xenstore.in
+++ b/tools/hotplug/Linux/launch-xenstore.in
@@ -54,6 +54,7 @@ test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && .
@CONFIG_DIR@/@CONFIG_LEAF
[ "$XENSTORETYPE" = "daemon" ] && {
[ -z "$XENSTORED_TRACE" ] || XENSTORED_ARGS="$XENSTORED_ARGS -T
@XEN_LOG_DIR@/xenstored-trace.log"
+ [ -z "$XENSTORED_MAX_N_DOMAINS" ] && XENSTORED_MAX_N_DOMAINS=32768
[ -z "$XENSTORED" ] && XENSTORED=@XENSTORED@
[ -x "$XENSTORED" ] || {
echo "No xenstored found"
@@ -63,6 +64,7 @@ test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && .
@CONFIG_DIR@/@CONFIG_LEAF
XS_OOM_SCORE=-$(($XENSTORED_OOM_MEM_THRESHOLD * 10))
rm -f @XEN_RUN_DIR@/xenstored.pid
+ N_FILES=$(($XENSTORED_MAX_N_DOMAINS * 5 + 100))
echo -n Starting $XENSTORED...
$XENSTORED --pid-file @XEN_RUN_DIR@/xenstored.pid $XENSTORED_ARGS
@@ -70,6 +72,7 @@ test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && .
@CONFIG_DIR@/@CONFIG_LEAF
systemd-notify --booted 2>/dev/null || timeout_xenstore $XENSTORED ||
exit 1
XS_PID=`cat @XEN_RUN_DIR@/xenstored.pid`
echo $XS_OOM_SCORE >/proc/$XS_PID/oom_score_adj
+ prlimit --pid $XS_PID --nofile=$N_FILES
exit 0
}
--
2.26.2
[PATCH v3 0/2] tools/xenstore: set resource limits of xenstored
Set some limits for xenstored in order to avoid it being killed by OOM killer, or to run out of file descriptors. Changes in V3: - make oom score configurable Changes in V2: - split into 2 patches - set limits from start script Juergen Gross (2): tools/xenstore: set oom score for xenstore daemon on Linux tools/xenstore: set open file descriptor limit for xenstored tools/hotplug/Linux/init.d/sysconfig.xencommons.in | 14 ++ tools/hotplug/Linux/launch-xenstore.in | 9 + 2 files changed, 23 insertions(+) -- 2.26.2
[PATCH v3 3/3] xen/blkfront: don't trust the backend response data blindly
Today blkfront will trust the backend to send only sane response data.
In order to avoid privilege escalations or crashes in case of malicious
backends verify the data to be within expected limits. Especially make
sure that the response always references an outstanding request.
Introduce a new state of the ring BLKIF_STATE_ERROR which will be
switched to in case an inconsistency is being detected. Recovering from
this state is possible only via removing and adding the virtual device
again (e.g. via a suspend/resume cycle).
Make all warning messages issued due to valid error responses rate
limited in order to avoid message floods being triggered by a malicious
backend.
Signed-off-by: Juergen Gross
Reviewed-by: Jan Beulich
Acked-by: Roger Pau Monné
---
V2:
- use READ_ONCE() for reading the producer index
- check validity of producer index only after memory barrier (Jan Beulich)
- use virt_rmb() as barrier (Jan Beulich)
V3:
- insert missing unlock in error case (kernel test robot)
- use %#x as format for printing wrong operation value (Roger Pau Monné)
---
drivers/block/xen-blkfront.c | 70 +++-
1 file changed, 53 insertions(+), 17 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index b7301006fb28..7a5e62fae171 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -80,6 +80,7 @@ enum blkif_state {
BLKIF_STATE_DISCONNECTED,
BLKIF_STATE_CONNECTED,
BLKIF_STATE_SUSPENDED,
+ BLKIF_STATE_ERROR,
};
struct grant {
@@ -89,6 +90,7 @@ struct grant {
};
enum blk_req_status {
+ REQ_PROCESSING,
REQ_WAITING,
REQ_DONE,
REQ_ERROR,
@@ -530,7 +532,7 @@ static unsigned long blkif_ring_get_request(struct
blkfront_ring_info *rinfo,
id = get_id_from_freelist(rinfo);
rinfo->shadow[id].request = req;
- rinfo->shadow[id].status = REQ_WAITING;
+ rinfo->shadow[id].status = REQ_PROCESSING;
rinfo->shadow[id].associated_id = NO_ASSOCIATED_ID;
rinfo->shadow[id].req.u.rw.id = id;
@@ -559,6 +561,7 @@ static int blkif_queue_discard_req(struct request *req,
struct blkfront_ring_inf
/* Copy the request to the ring page. */
*final_ring_req = *ring_req;
+ rinfo->shadow[id].status = REQ_WAITING;
return 0;
}
@@ -834,8 +837,11 @@ static int blkif_queue_rw_req(struct request *req, struct
blkfront_ring_info *ri
/* Copy request(s) to the ring page. */
*final_ring_req = *ring_req;
- if (unlikely(require_extra_req))
+ rinfo->shadow[id].status = REQ_WAITING;
+ if (unlikely(require_extra_req)) {
*final_extra_ring_req = *extra_ring_req;
+ rinfo->shadow[extra_id].status = REQ_WAITING;
+ }
if (new_persistent_gnts)
gnttab_free_grant_references(setup.gref_head);
@@ -1359,8 +1365,8 @@ static enum blk_req_status blkif_rsp_to_req_status(int
rsp)
static int blkif_get_final_status(enum blk_req_status s1,
enum blk_req_status s2)
{
- BUG_ON(s1 == REQ_WAITING);
- BUG_ON(s2 == REQ_WAITING);
+ BUG_ON(s1 < REQ_DONE);
+ BUG_ON(s2 < REQ_DONE);
if (s1 == REQ_ERROR || s2 == REQ_ERROR)
return BLKIF_RSP_ERROR;
@@ -1393,7 +1399,7 @@ static bool blkif_completion(unsigned long *id,
s->status = blkif_rsp_to_req_status(bret->status);
/* Wait the second response if not yet here. */
- if (s2->status == REQ_WAITING)
+ if (s2->status < REQ_DONE)
return false;
bret->status = blkif_get_final_status(s->status,
@@ -1512,11 +1518,17 @@ static irqreturn_t blkif_interrupt(int irq, void
*dev_id)
spin_lock_irqsave(&rinfo->ring_lock, flags);
again:
- rp = rinfo->ring.sring->rsp_prod;
- rmb(); /* Ensure we see queued responses up to 'rp'. */
+ rp = READ_ONCE(rinfo->ring.sring->rsp_prod);
+ virt_rmb(); /* Ensure we see queued responses up to 'rp'. */
+ if (RING_RESPONSE_PROD_OVERFLOW(&rinfo->ring, rp)) {
+ pr_alert("%s: illegal number of responses %u\n",
+info->gd->disk_name, rp - rinfo->ring.rsp_cons);
+ goto err;
+ }
for (i = rinfo->ring.rsp_cons; i != rp; i++) {
unsigned long id;
+ unsigned int op;
RING_COPY_RESPONSE(&rinfo->ring, i, &bret);
id = bret.id;
@@ -1527,14 +1539,28 @@ static irqreturn_t blkif_interrupt(int irq, void
*dev_id)
* look in get_id_from_freelist.
*/
if (id >= BLK_RING_SIZE(info)) {
- WARN(1, "%s: response to %s has incorrect id (%ld)\n",
-info->gd->disk_name, op_name(bret.operation), id);
- /* We can't safely get the 'struct request' as
-
[PATCH v3 2/3] xen/blkfront: don't take local copy of a request from the ring page
In order to avoid a malicious backend being able to influence the local
copy of a request build the request locally first and then copy it to
the ring page instead of doing it the other way round as today.
Signed-off-by: Juergen Gross
Reviewed-by: Jan Beulich
Acked-by: Roger Pau Monné
---
V2:
- init variable to avoid potential compiler warning (Jan Beulich)
---
drivers/block/xen-blkfront.c | 25 +++--
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 15e840287734..b7301006fb28 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -533,7 +533,7 @@ static unsigned long blkif_ring_get_request(struct
blkfront_ring_info *rinfo,
rinfo->shadow[id].status = REQ_WAITING;
rinfo->shadow[id].associated_id = NO_ASSOCIATED_ID;
- (*ring_req)->u.rw.id = id;
+ rinfo->shadow[id].req.u.rw.id = id;
return id;
}
@@ -541,11 +541,12 @@ static unsigned long blkif_ring_get_request(struct
blkfront_ring_info *rinfo,
static int blkif_queue_discard_req(struct request *req, struct
blkfront_ring_info *rinfo)
{
struct blkfront_info *info = rinfo->dev_info;
- struct blkif_request *ring_req;
+ struct blkif_request *ring_req, *final_ring_req;
unsigned long id;
/* Fill out a communications ring structure. */
- id = blkif_ring_get_request(rinfo, req, &ring_req);
+ id = blkif_ring_get_request(rinfo, req, &final_ring_req);
+ ring_req = &rinfo->shadow[id].req;
ring_req->operation = BLKIF_OP_DISCARD;
ring_req->u.discard.nr_sectors = blk_rq_sectors(req);
@@ -556,8 +557,8 @@ static int blkif_queue_discard_req(struct request *req,
struct blkfront_ring_inf
else
ring_req->u.discard.flag = 0;
- /* Keep a private copy so we can reissue requests when recovering. */
- rinfo->shadow[id].req = *ring_req;
+ /* Copy the request to the ring page. */
+ *final_ring_req = *ring_req;
return 0;
}
@@ -690,6 +691,7 @@ static int blkif_queue_rw_req(struct request *req, struct
blkfront_ring_info *ri
{
struct blkfront_info *info = rinfo->dev_info;
struct blkif_request *ring_req, *extra_ring_req = NULL;
+ struct blkif_request *final_ring_req, *final_extra_ring_req = NULL;
unsigned long id, extra_id = NO_ASSOCIATED_ID;
bool require_extra_req = false;
int i;
@@ -734,7 +736,8 @@ static int blkif_queue_rw_req(struct request *req, struct
blkfront_ring_info *ri
}
/* Fill out a communications ring structure. */
- id = blkif_ring_get_request(rinfo, req, &ring_req);
+ id = blkif_ring_get_request(rinfo, req, &final_ring_req);
+ ring_req = &rinfo->shadow[id].req;
num_sg = blk_rq_map_sg(req->q, req, rinfo->shadow[id].sg);
num_grant = 0;
@@ -785,7 +788,9 @@ static int blkif_queue_rw_req(struct request *req, struct
blkfront_ring_info *ri
ring_req->u.rw.nr_segments = num_grant;
if (unlikely(require_extra_req)) {
extra_id = blkif_ring_get_request(rinfo, req,
- &extra_ring_req);
+
&final_extra_ring_req);
+ extra_ring_req = &rinfo->shadow[extra_id].req;
+
/*
* Only the first request contains the scatter-gather
* list.
@@ -827,10 +832,10 @@ static int blkif_queue_rw_req(struct request *req, struct
blkfront_ring_info *ri
if (setup.segments)
kunmap_atomic(setup.segments);
- /* Keep a private copy so we can reissue requests when recovering. */
- rinfo->shadow[id].req = *ring_req;
+ /* Copy request(s) to the ring page. */
+ *final_ring_req = *ring_req;
if (unlikely(require_extra_req))
- rinfo->shadow[extra_id].req = *extra_ring_req;
+ *final_extra_ring_req = *extra_ring_req;
if (new_persistent_gnts)
gnttab_free_grant_references(setup.gref_head);
--
2.26.2
[PATCH v3 1/3] xen/blkfront: read response from backend only once
In order to avoid problems in case the backend is modifying a response
on the ring page while the frontend has already seen it, just read the
response into a local buffer in one go and then operate on that buffer
only.
Signed-off-by: Juergen Gross
Reviewed-by: Jan Beulich
Acked-by: Roger Pau Monné
---
drivers/block/xen-blkfront.c | 35 ++-
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index d83fee21f6c5..15e840287734 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -1496,7 +1496,7 @@ static bool blkif_completion(unsigned long *id,
static irqreturn_t blkif_interrupt(int irq, void *dev_id)
{
struct request *req;
- struct blkif_response *bret;
+ struct blkif_response bret;
RING_IDX i, rp;
unsigned long flags;
struct blkfront_ring_info *rinfo = (struct blkfront_ring_info *)dev_id;
@@ -1513,8 +1513,9 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id)
for (i = rinfo->ring.rsp_cons; i != rp; i++) {
unsigned long id;
- bret = RING_GET_RESPONSE(&rinfo->ring, i);
- id = bret->id;
+ RING_COPY_RESPONSE(&rinfo->ring, i, &bret);
+ id = bret.id;
+
/*
* The backend has messed up and given us an id that we would
* never have given to it (we stamp it up to BLK_RING_SIZE -
@@ -1522,39 +1523,39 @@ static irqreturn_t blkif_interrupt(int irq, void
*dev_id)
*/
if (id >= BLK_RING_SIZE(info)) {
WARN(1, "%s: response to %s has incorrect id (%ld)\n",
-info->gd->disk_name, op_name(bret->operation), id);
+info->gd->disk_name, op_name(bret.operation), id);
/* We can't safely get the 'struct request' as
* the id is busted. */
continue;
}
req = rinfo->shadow[id].request;
- if (bret->operation != BLKIF_OP_DISCARD) {
+ if (bret.operation != BLKIF_OP_DISCARD) {
/*
* We may need to wait for an extra response if the
* I/O request is split in 2
*/
- if (!blkif_completion(&id, rinfo, bret))
+ if (!blkif_completion(&id, rinfo, &bret))
continue;
}
if (add_id_to_freelist(rinfo, id)) {
WARN(1, "%s: response to %s (id %ld) couldn't be
recycled!\n",
-info->gd->disk_name, op_name(bret->operation), id);
+info->gd->disk_name, op_name(bret.operation), id);
continue;
}
- if (bret->status == BLKIF_RSP_OKAY)
+ if (bret.status == BLKIF_RSP_OKAY)
blkif_req(req)->error = BLK_STS_OK;
else
blkif_req(req)->error = BLK_STS_IOERR;
- switch (bret->operation) {
+ switch (bret.operation) {
case BLKIF_OP_DISCARD:
- if (unlikely(bret->status == BLKIF_RSP_EOPNOTSUPP)) {
+ if (unlikely(bret.status == BLKIF_RSP_EOPNOTSUPP)) {
struct request_queue *rq = info->rq;
printk(KERN_WARNING "blkfront: %s: %s op
failed\n",
- info->gd->disk_name,
op_name(bret->operation));
+ info->gd->disk_name,
op_name(bret.operation));
blkif_req(req)->error = BLK_STS_NOTSUPP;
info->feature_discard = 0;
info->feature_secdiscard = 0;
@@ -1564,15 +1565,15 @@ static irqreturn_t blkif_interrupt(int irq, void
*dev_id)
break;
case BLKIF_OP_FLUSH_DISKCACHE:
case BLKIF_OP_WRITE_BARRIER:
- if (unlikely(bret->status == BLKIF_RSP_EOPNOTSUPP)) {
+ if (unlikely(bret.status == BLKIF_RSP_EOPNOTSUPP)) {
printk(KERN_WARNING "blkfront: %s: %s op
failed\n",
- info->gd->disk_name,
op_name(bret->operation));
+ info->gd->disk_name,
op_name(bret.operation));
blkif_req(req)->error = BLK_STS_NOTSUPP;
}
- if (unlikely(bret->status == BLKIF_RSP_ERROR &&
+ if (unlikely(bret.status == BLKIF_RSP_ERROR &&
rinfo->shadow[id].req.u.rw.nr_segments ==
0)) {
[PATCH v3 0/3] xen: harden blkfront against malicious backends
Xen backends of para-virtualized devices can live in dom0 kernel, dom0 user land, or in a driver domain. This means that a backend might reside in a less trusted environment than the Xen core components, so a backend should not be able to do harm to a Xen guest (it can still mess up I/O data, but it shouldn't be able to e.g. crash a guest by other means or cause a privilege escalation in the guest). Unfortunately blkfront in the Linux kernel is fully trusting its backend. This series is fixing blkfront in this regard. It was discussed to handle this as a security problem, but the topic was discussed in public before, so it isn't a real secret. It should be mentioned that a similar series has been posted some years ago by Marek Marczykowski-Górecki, but this series has not been applied due to a Xen header not having been available in the Xen git repo at that time. Additionally my series is fixing some more DoS cases. Changes in V3: - patch 3: insert missing unlock in error case (kernel test robot) - patch 3: use %#x as format for printing wrong operation value (Roger Pau Monné) Changes in V2: - put blkfront patches into own series - some minor comments addressed Juergen Gross (3): xen/blkfront: read response from backend only once xen/blkfront: don't take local copy of a request from the ring page xen/blkfront: don't trust the backend response data blindly drivers/block/xen-blkfront.c | 126 +++ 1 file changed, 84 insertions(+), 42 deletions(-) -- 2.26.2
Re: [PATCH v2 3/3] xen/blkfront: don't trust the backend response data blindly
On 30.07.21 12:08, Juergen Gross wrote: On 08.07.21 14:43, Juergen Gross wrote: Today blkfront will trust the backend to send only sane response data. In order to avoid privilege escalations or crashes in case of malicious backends verify the data to be within expected limits. Especially make sure that the response always references an outstanding request. Introduce a new state of the ring BLKIF_STATE_ERROR which will be switched to in case an inconsistency is being detected. Recovering from this state is possible only via removing and adding the virtual device again (e.g. via a suspend/resume cycle). Signed-off-by: Juergen Gross Any comments for this patch? Please ignore this request for comments, I already got some. Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH] xen/lib: Fix strcmp() and strncmp()
On 27/07/2021 19:47, Jane Malalane wrote:
> diff --git a/xen/lib/strcmp.c b/xen/lib/strcmp.c
> index 465f1c4191..f85c1e8741 100644
> --- a/xen/lib/strcmp.c
> +++ b/xen/lib/strcmp.c
> @@ -11,14 +11,16 @@
> */
> int (strcmp)(const char *cs, const char *ct)
> {
> - register signed char __res;
> + unsigned char *csu = (unsigned char *)cs;
> + unsigned char *ctu = (unsigned char *)ct;
So there was actually one final thing, but it is holiday season, hence
the lack of replies from others.
We should not be casting away const-ness on the pointers, because that
is undefined behaviour and compilers are starting to warn about it.
Therefore, we want something like:
const unsigned char *csu = (const unsigned char *)cs;
~Andrew
Re: [PATCH v2 3/3] xen/blkfront: don't trust the backend response data blindly
On 08.07.21 14:43, Juergen Gross wrote:
Today blkfront will trust the backend to send only sane response data.
In order to avoid privilege escalations or crashes in case of malicious
backends verify the data to be within expected limits. Especially make
sure that the response always references an outstanding request.
Introduce a new state of the ring BLKIF_STATE_ERROR which will be
switched to in case an inconsistency is being detected. Recovering from
this state is possible only via removing and adding the virtual device
again (e.g. via a suspend/resume cycle).
Signed-off-by: Juergen Gross
Any comments for this patch?
Juergen
---
V2:
- use READ_ONCE() for reading the producer index
- check validity of producer index only after memory barrier (Jan Beulich)
- use virt_rmb() as barrier (Jan Beulich)
---
drivers/block/xen-blkfront.c | 66 ++--
1 file changed, 49 insertions(+), 17 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 80701860870a..ecdbb0381b4c 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -80,6 +80,7 @@ enum blkif_state {
BLKIF_STATE_DISCONNECTED,
BLKIF_STATE_CONNECTED,
BLKIF_STATE_SUSPENDED,
+ BLKIF_STATE_ERROR,
};
struct grant {
@@ -89,6 +90,7 @@ struct grant {
};
enum blk_req_status {
+ REQ_PROCESSING,
REQ_WAITING,
REQ_DONE,
REQ_ERROR,
@@ -543,7 +545,7 @@ static unsigned long blkif_ring_get_request(struct
blkfront_ring_info *rinfo,
id = get_id_from_freelist(rinfo);
rinfo->shadow[id].request = req;
- rinfo->shadow[id].status = REQ_WAITING;
+ rinfo->shadow[id].status = REQ_PROCESSING;
rinfo->shadow[id].associated_id = NO_ASSOCIATED_ID;
rinfo->shadow[id].req.u.rw.id = id;
@@ -572,6 +574,7 @@ static int blkif_queue_discard_req(struct request *req,
struct blkfront_ring_inf
/* Copy the request to the ring page. */
*final_ring_req = *ring_req;
+ rinfo->shadow[id].status = REQ_WAITING;
return 0;
}
@@ -847,8 +850,11 @@ static int blkif_queue_rw_req(struct request *req, struct
blkfront_ring_info *ri
/* Copy request(s) to the ring page. */
*final_ring_req = *ring_req;
- if (unlikely(require_extra_req))
+ rinfo->shadow[id].status = REQ_WAITING;
+ if (unlikely(require_extra_req)) {
*final_extra_ring_req = *extra_ring_req;
+ rinfo->shadow[extra_id].status = REQ_WAITING;
+ }
if (new_persistent_gnts)
gnttab_free_grant_references(setup.gref_head);
@@ -1402,8 +1408,8 @@ static enum blk_req_status blkif_rsp_to_req_status(int
rsp)
static int blkif_get_final_status(enum blk_req_status s1,
enum blk_req_status s2)
{
- BUG_ON(s1 == REQ_WAITING);
- BUG_ON(s2 == REQ_WAITING);
+ BUG_ON(s1 < REQ_DONE);
+ BUG_ON(s2 < REQ_DONE);
if (s1 == REQ_ERROR || s2 == REQ_ERROR)
return BLKIF_RSP_ERROR;
@@ -1436,7 +1442,7 @@ static bool blkif_completion(unsigned long *id,
s->status = blkif_rsp_to_req_status(bret->status);
/* Wait the second response if not yet here. */
- if (s2->status == REQ_WAITING)
+ if (s2->status < REQ_DONE)
return false;
bret->status = blkif_get_final_status(s->status,
@@ -1555,11 +1561,17 @@ static irqreturn_t blkif_interrupt(int irq, void
*dev_id)
spin_lock_irqsave(&rinfo->ring_lock, flags);
again:
- rp = rinfo->ring.sring->rsp_prod;
- rmb(); /* Ensure we see queued responses up to 'rp'. */
+ rp = READ_ONCE(rinfo->ring.sring->rsp_prod);
+ virt_rmb(); /* Ensure we see queued responses up to 'rp'. */
+ if (RING_RESPONSE_PROD_OVERFLOW(&rinfo->ring, rp)) {
+ pr_alert("%s: illegal number of responses %u\n",
+info->gd->disk_name, rp - rinfo->ring.rsp_cons);
+ goto err;
+ }
for (i = rinfo->ring.rsp_cons; i != rp; i++) {
unsigned long id;
+ unsigned int op;
RING_COPY_RESPONSE(&rinfo->ring, i, &bret);
id = bret.id;
@@ -1570,14 +1582,28 @@ static irqreturn_t blkif_interrupt(int irq, void
*dev_id)
* look in get_id_from_freelist.
*/
if (id >= BLK_RING_SIZE(info)) {
- WARN(1, "%s: response to %s has incorrect id (%ld)\n",
-info->gd->disk_name, op_name(bret.operation), id);
- /* We can't safely get the 'struct request' as
-* the id is busted. */
- continue;
+ pr_alert("%s: response has incorrect id (%ld)\n",
+info->gd->disk_name, id);
+ goto err;
}
+ if (rinfo->shadow[id].
Re: [PATCH] tools/xenstored: Fix off-by-one in dump_state_nodes() [and 3 more messages]
Julien Grall writes ("[PATCH] tools/xenstored: Fix off-by-one in
dump_state_nodes()"):
> The maximum path length supported by Xenstored protocol is
> XENSTORE_ABS_PATH_MAX (i.e 3072). This doesn't take into account the
> NUL at the end of the path.
...
Julien Grall writes ("[PATCH] tools/xenstored: Propagate correctly the error
message from lu_start()"):
> lu_start() will only set errno when it returns NULL. For all the
> other cases, the value is unknown.
Thanks, and to Juergen for the reviews. Pushed.
Ian.
Re: [PATCH] xen/lib: Fix strcmp() and strncmp()
Jane Malalane writes ("Re: [PATCH] xen/lib: Fix strcmp() and strncmp()"):
> On 28/07/2021 11:42, Ian Jackson wrote:
> What are the practical effects of this bug ? AFAICT in the hypervisor
> code all the call sites simply test for zero/nonzero.
...
> This fix was just to make the code spec compliant and mainly for practice
> as I'm currently being introduced to Xen.
OK, great. As I say it looks correct to me. I just wanted to make
sure I wasn't missing anything.
So,
Reviewed-by: Ian Jackson
and I will queue this.
Ian.
[PATCH v2 2/2] xen: rename wrong named pfn related variables
There are some variables in Xen specific coding which names imply them
holding a PFN, while in reality they are containing numbers of PFNs.
Rename them accordingly.
Signed-off-by: Juergen Gross
---
V2:
- adjust comment (Jan Beulich)
---
arch/x86/include/asm/xen/page.h | 4 ++--
arch/x86/xen/p2m.c | 33 ++---
arch/x86/xen/setup.c| 2 +-
3 files changed, 21 insertions(+), 18 deletions(-)
diff --git a/arch/x86/include/asm/xen/page.h b/arch/x86/include/asm/xen/page.h
index 1a162e559753..3590d6bf42c7 100644
--- a/arch/x86/include/asm/xen/page.h
+++ b/arch/x86/include/asm/xen/page.h
@@ -51,7 +51,7 @@ extern unsigned long *machine_to_phys_mapping;
extern unsigned long machine_to_phys_nr;
extern unsigned long *xen_p2m_addr;
extern unsigned long xen_p2m_size;
-extern unsigned long xen_max_p2m_pfn;
+extern unsigned long xen_p2m_max_size;
extern int xen_alloc_p2m_entry(unsigned long pfn);
@@ -144,7 +144,7 @@ static inline unsigned long __pfn_to_mfn(unsigned long pfn)
if (pfn < xen_p2m_size)
mfn = xen_p2m_addr[pfn];
- else if (unlikely(pfn < xen_max_p2m_pfn))
+ else if (unlikely(pfn < xen_p2m_max_size))
return get_phys_to_machine(pfn);
else
return IDENTITY_FRAME(pfn);
diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c
index 5e6e236977c7..d75d9e077d13 100644
--- a/arch/x86/xen/p2m.c
+++ b/arch/x86/xen/p2m.c
@@ -95,8 +95,8 @@ unsigned long *xen_p2m_addr __read_mostly;
EXPORT_SYMBOL_GPL(xen_p2m_addr);
unsigned long xen_p2m_size __read_mostly;
EXPORT_SYMBOL_GPL(xen_p2m_size);
-unsigned long xen_max_p2m_pfn __read_mostly;
-EXPORT_SYMBOL_GPL(xen_max_p2m_pfn);
+unsigned long xen_p2m_max_size __read_mostly;
+EXPORT_SYMBOL_GPL(xen_p2m_max_size);
#ifdef CONFIG_XEN_MEMORY_HOTPLUG_LIMIT
#define P2M_LIMIT CONFIG_XEN_MEMORY_HOTPLUG_LIMIT
@@ -120,8 +120,10 @@ static pte_t *p2m_identity_pte;
* Used to set HYPERVISOR_shared_info->arch.max_pfn so the toolstack
* can avoid scanning the whole P2M (which may be sized to account for
* hotplugged memory).
+ *
+ * All populated P2M entries have an index lower than xen_p2m_pfn_limit.
*/
-static unsigned long xen_p2m_last_pfn;
+static unsigned long xen_p2m_pfn_limit;
static inline unsigned p2m_top_index(unsigned long pfn)
{
@@ -239,7 +241,7 @@ void __ref xen_build_mfn_list_list(void)
p2m_mid_mfn_init(p2m_mid_missing_mfn, p2m_missing);
}
- for (pfn = 0; pfn < xen_max_p2m_pfn && pfn < MAX_P2M_PFN;
+ for (pfn = 0; pfn < xen_p2m_max_size && pfn < MAX_P2M_PFN;
pfn += P2M_PER_PAGE) {
topidx = p2m_top_index(pfn);
mididx = p2m_mid_index(pfn);
@@ -284,7 +286,7 @@ void xen_setup_mfn_list_list(void)
else
HYPERVISOR_shared_info->arch.pfn_to_mfn_frame_list_list =
virt_to_mfn(p2m_top_mfn);
- HYPERVISOR_shared_info->arch.max_pfn = xen_p2m_last_pfn;
+ HYPERVISOR_shared_info->arch.max_pfn = xen_p2m_pfn_limit;
HYPERVISOR_shared_info->arch.p2m_generation = 0;
HYPERVISOR_shared_info->arch.p2m_vaddr = (unsigned long)xen_p2m_addr;
HYPERVISOR_shared_info->arch.p2m_cr3 =
@@ -302,7 +304,7 @@ void __init xen_build_dynamic_phys_to_machine(void)
for (pfn = xen_start_info->nr_pages; pfn < xen_p2m_size; pfn++)
xen_p2m_addr[pfn] = INVALID_P2M_ENTRY;
- xen_max_p2m_pfn = xen_p2m_size;
+ xen_p2m_max_size = xen_p2m_size;
}
#define P2M_TYPE_IDENTITY 0
@@ -353,7 +355,7 @@ static void __init xen_rebuild_p2m_list(unsigned long *p2m)
pfn_pte(PFN_DOWN(__pa(p2m_identity)), PAGE_KERNEL_RO));
}
- for (pfn = 0; pfn < xen_max_p2m_pfn; pfn += chunk) {
+ for (pfn = 0; pfn < xen_p2m_max_size; pfn += chunk) {
/*
* Try to map missing/identity PMDs or p2m-pages if possible.
* We have to respect the structure of the mfn_list_list
@@ -413,21 +415,22 @@ void __init xen_vmalloc_p2m_tree(void)
static struct vm_struct vm;
unsigned long p2m_limit;
- xen_p2m_last_pfn = xen_max_p2m_pfn;
+ xen_p2m_pfn_limit = xen_p2m_max_size;
p2m_limit = (phys_addr_t)P2M_LIMIT * 1024 * 1024 * 1024 / PAGE_SIZE;
vm.flags = VM_ALLOC;
- vm.size = ALIGN(sizeof(unsigned long) * max(xen_max_p2m_pfn, p2m_limit),
+ vm.size = ALIGN(sizeof(unsigned long) *
+ max(xen_p2m_max_size, p2m_limit),
PMD_SIZE * PMDS_PER_MID_PAGE);
vm_area_register_early(&vm, PMD_SIZE * PMDS_PER_MID_PAGE);
pr_notice("p2m virtual area at %p, size is %lx\n", vm.addr, vm.size);
- xen_max_p2m_pfn = vm.size / sizeof(unsigned long);
+ xen_p2m_max_size = vm.size / sizeof(unsigned long);
xen_rebuild_p2m_list(vm.addr);
xen_p2m_addr = vm.addr;
- xen_p2m_size = xen_max_p2m_pfn;
+ x
[PATCH v2 1/2] xen: fix setting of max_pfn in shared_info
Xen PV guests are specifying the highest used PFN via the max_pfn
field in shared_info. This value is used by the Xen tools when saving
or migrating the guest.
Unfortunately this field is misnamed, as in reality it is specifying
the number of pages (including any memory holes) of the guest, so it
is the highest used PFN + 1. Renaming isn't possible, as this is a
public Xen hypervisor interface which needs to be kept stable.
The kernel will set the value correctly initially at boot time, but
when adding more pages (e.g. due to memory hotplug or ballooning) a
real PFN number is stored in max_pfn. This is done when expanding the
p2m array, and the PFN stored there is even possibly wrong, as it
should be the last possible PFN of the just added P2M frame, and not
one which led to the P2M expansion.
Fix that by setting shared_info->max_pfn to the last possible PFN + 1.
Fixes: 98dd166ea3a3c3 ("x86/xen/p2m: hint at the last populated P2M entry")
Cc: sta...@vger.kernel.org
Signed-off-by: Juergen Gross
Reviewed-by: Jan Beulich
---
arch/x86/xen/p2m.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c
index ac06ca32e9ef..5e6e236977c7 100644
--- a/arch/x86/xen/p2m.c
+++ b/arch/x86/xen/p2m.c
@@ -618,8 +618,8 @@ int xen_alloc_p2m_entry(unsigned long pfn)
}
/* Expanded the p2m? */
- if (pfn > xen_p2m_last_pfn) {
- xen_p2m_last_pfn = pfn;
+ if (pfn >= xen_p2m_last_pfn) {
+ xen_p2m_last_pfn = ALIGN(pfn + 1, P2M_PER_PAGE);
HYPERVISOR_shared_info->arch.max_pfn = xen_p2m_last_pfn;
}
--
2.26.2
[PATCH v2 0/2] xen: fix max_pfn handling for pv guests
Fix some bad naming and setting of max_pfn related variables. Juergen Gross (2): xen: fix setting of max_pfn in shared_info xen: rename wrong named pfn related variables arch/x86/include/asm/xen/page.h | 4 ++-- arch/x86/xen/p2m.c | 33 ++--- arch/x86/xen/setup.c| 2 +- 3 files changed, 21 insertions(+), 18 deletions(-) -- 2.26.2
Re: [PATCH] stubdom: foreignmemory: Fix build after 0dbb4be739c5
On 7/27/21 4:36 PM, Andrew Cooper wrote: > On 16/07/2021 19:28, Costin Lupu wrote: >> On 7/13/21 6:20 PM, Juergen Gross wrote: >>> On 13.07.21 17:15, Julien Grall wrote: Hi Juergen, On 13/07/2021 16:09, Juergen Gross wrote: > On 13.07.21 16:38, Julien Grall wrote: >> Hi Juergen, >> >> On 13/07/2021 15:23, Juergen Gross wrote: >>> On 13.07.21 16:19, Julien Grall wrote: Hi Jan, On 13/07/2021 15:14, Jan Beulich wrote: >> And I don't think it should be named XC_PAGE_*, but rather >> XEN_PAGE_*. > Even that doesn't seem right to me, at least in principle. There > shouldn't > be a build time setting when it may vary at runtime. IOW on Arm I > think a > runtime query to the hypervisor would be needed instead. Yes, we want to be able to use the same userspace/OS without rebuilding to a specific hypervisor page size. >>> This define is used for accessing data of other domains. See the >>> define >>> for XEN_PAGE_SIZE in xen/include/public/io/ring.h >>> >>> So it should be a constant (minimal) page size for all hypervisors and >>> guests of an architecture. >> Do you mean the maximum rather than minimal? If you use the minimal >> (4KB), then you would not be able to map the page in the stage-2 if >> the hypervisor is using 64KB. > But this would mean that the current solution to use XC_PAGE_SIZE is > wrong, as this is 4k. The existing ABI is implicitely based on using the hypervisor page granularity (currently 4KB). There is really no way we can support existing guest on 64KB hypervisor. But if we were going to break them, then we should consider to do one of the following option: 1) use 64KB page granularity for ABI 2) query the hypervisor page granularity at runtime The ideal is 2) because it is more scalable for the future. We also need to consider to extend the PV protocol so the backend and frontend can agree on the page size. >>> I absolutely agree, but my suggestion was to help finding a proper way >>> to cleanup the current interface mess. And this should be done the way >>> I suggested IMO. >>> >>> A later interface extension for future guests can still be done on top >>> of that. >> Alright, let's have a little recap to see if I got it right and to agree >> on the next steps. There are 2 proposed solutions, let's say a static >> one and a dynamic one. >> >> 1) Static solution (proposed by Juergen) >> - We define XEN_PAGE_* values in a xen/include/public/arch-*/*.h header. >> - Q: Should we define a new header for that? page.h or page_size.h are >> ok as new filenames? >> >> Pros: >> - We fix the interfaces mess and we can get rid of xenctrl lib >> dependency for some of the libs that need only the XEN_PAGE_* definitions. >> - It's faster to implement, with fewer changes. >> >> Cons: >> - Well, it's static, it doesn't allow the hypervisor to provide >> different values for different guests. >> >> >> 2) Dynamic solution (proposed by Jan and Julien) >> We get the value(s) by calling a hypcall, probably as a query related to >> some guest domain. >> >> Pros: >> - It's dynamic and scalable. We would support different values for >> different guests. >> >> Cons: >> - More difficult to implement. It changes the paradigm in the toolstack >> libs, every occurrence of XC_PAGE_* would have to be amended. Moreover, >> we might want to make the hypcall once and save the value for later >> (probably several toolstack structures should be extended for that) >> >> >> I searched for the occurrences of XC_PAGE_* in the toolstack libs and >> it's a *lot* of them. IMHO I think we should pick the static solution >> for now, considering that it would be faster to implement. Please let me >> know if this is OK or not. Any comments are appreciated. > > The immediate problem needing fixing is the stable libraries inclusion > of unstable headers - specifically, the inclusion of . > > Juergen's proposal moves the existing constant to a more appropriate > location, and specifically, a location where its value is stable. > > It does not change the ABI. It merely demonstrates that the existing > ABI is broken, and thus is absolutely a step in the right direction. > > This is the approach you should take in the short term, and needs > sorting before 4.16 ships. > > > The dynamic solution, while preferable in the longterm, is far more > complicated than even described thus far, and is not as simple as just > having a hypercall and using that value. > > Among other things, it requires coordination with the dom0 kernel as to > its pagetable setup, and with Xen's choice of pagetable size for dom0, > which may not be the same as domU's. It is a large quantity of work, > very invasive to the existing APIs/ABIs, and stands no chance at all of > being ready for 4.16. Thanks for clearing this,
Re: [PATCH 2/2] xen: rename wrong named pfn related variables
On 16.06.21 12:43, Juergen Gross wrote: On 16.06.21 11:56, Jan Beulich wrote: On 16.06.2021 09:30, Juergen Gross wrote: --- a/arch/x86/xen/p2m.c +++ b/arch/x86/xen/p2m.c @@ -95,8 +95,8 @@ unsigned long *xen_p2m_addr __read_mostly; EXPORT_SYMBOL_GPL(xen_p2m_addr); unsigned long xen_p2m_size __read_mostly; EXPORT_SYMBOL_GPL(xen_p2m_size); -unsigned long xen_max_p2m_pfn __read_mostly; -EXPORT_SYMBOL_GPL(xen_max_p2m_pfn); +unsigned long xen_p2m_max_size __read_mostly; +EXPORT_SYMBOL_GPL(xen_p2m_max_size); Instead of renaming the exported variable (which will break consumers anyway), how about dropping the apparently unneeded export at this occasion? Why do you think it isn't needed? It is being referenced via the inline function __pfn_to_mfn() in arch/x86/include/asm/xen/page.h. And __pfn_to_mfn() is used via lots of other inline functions and macros. Further it looks to me as if xen_p2m_size and this variable were actually always kept in sync, so I'd like to put up the question of dropping one of the two. Hmm, should be possible, yes. Looking into this it seems this is not possible. xen_p2m_size always holds the number of p2m entries in the p2m table, including invalid ones at the end. xen_p2m_pfn_limit however contains the (rounded up) index after the last valid p2m entry. Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
[libvirt test] 164050: regressions - FAIL
flight 164050 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/164050/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-armhf-libvirt 6 libvirt-buildfail REGR. vs. 151777 build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 151777 build-arm64-libvirt 6 libvirt-buildfail REGR. vs. 151777 build-i386-libvirt6 libvirt-buildfail REGR. vs. 151777 Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a version targeted for testing: libvirt bae39ea87133aed886699a423a7ccce2a8f8fd32 baseline version: libvirt 2c846fa6bcc11929c9fb857a22430fb9945654ad Last test of basis 151777 2020-07-10 04:19:19 Z 385 days Failing since151818 2020-07-11 04:18:52 Z 384 days 376 attempts Testing same since 164050 2021-07-30 04:20:05 Z0 days1 attempts People who touched revisions under test: Adolfo Jayme Barrientos Aleksandr Alekseev Aleksei Zakharov Andika Triwidada Andrea Bolognani Balázs Meskó Barrett Schonefeld Bastian Germann Bastien Orivel BiaoXiang Ye Bihong Yu Binfeng Wu Bjoern Walk Boris Fiuczynski Brian Turek Bruno Haible Chris Mayo Christian Ehrhardt Christian Kirbach Christian Schoenebeck Cole Robinson Collin Walling Cornelia Huck Cédric Bosdonnat Côme Borsoi Daniel Henrique Barboza Daniel Letai Daniel P. Berrange Daniel P. Berrangé Didik Supriadi Dmytro Linkin Eiichi Tsukata Eric Farman Erik Skultety Fabian Affolter Fabian Freyer Fabiano Fidêncio Fangge Jin Farhan Ali Fedora Weblate Translation gongwei Guoyi Tu Göran Uddeborg Halil Pasic Han Han Hao Wang Hela Basa Helmut Grohne Ian Wienand Jakob Meng Jamie Strandboge Jamie Strandboge Jan Kuparinen Jean-Baptiste Holcroft Jia Zhou Jianan Gao Jim Fehlig Jin Yan Jinsheng Zhang Jiri Denemark John Ferlan Jonathan Watt Jonathon Jongsma Julio Faracco Justin Gatzen Ján Tomko Kashyap Chamarthy Kevin Locke Kristina Hanicova Laine Stump Laszlo Ersek Lee Yarwood Liao Pingfang Lin Ma Lin Ma Lin Ma Liu Yiding Luke Yue Luyao Zhong Marc Hartmayer Marc-André Lureau Marek Marczykowski-Górecki Markus Schade Martin Kletzander Masayoshi Mizuma Matej Cepl Matt Coleman Matt Coleman Mauro Matteo Cascella Meina Li Michal Privoznik Michał Smyk Milo Casagrande Moshe Levi Muha Aliss Nathan Neal Gompa Nick Shyrokovskiy Nickys Music Group Nico Pache Nikolay Shirokovskiy Olaf Hering Olesya Gerasimenko Orion Poplawski Pany Patrick Magauran Paulo de Rezende Pinatti Pavel Hrdina Peng Liang Peter Krempa Pino Toscano Pino Toscano Piotr Drąg Prathamesh Chavan Richard W.M. Jones Ricky Tigg Roman Bogorodskiy Roman Bolshakov Ryan Gahagan Ryan Schmidt Sam Hartman Scott Shambarger Sebastian Mitterle SeongHyun Jo Shalini Chellathurai Saroja Shaojun Yang Shi Lei simmon Simon Chopin Simon Gaiser Stefan Bader Stefan Berger Stefan Berger Stefan Hajnoczi Szymon Scholz Thomas Huth Tim Wiederhake Tomáš Golembiovský Tomáš Janoušek Tuguoyi Ville Skyttä Vinayak Kale Wang Xin WangJian Weblate Wei Liu Wei Liu William Douglas Yalei Li <274268...@qq.com> Yalei Li Yang Fei Yang Hang Yanqiu Zhang Yaroslav Kargin Yi Li Yi Wang Yuri Chornoivan Zbigniew Jędrzejewski-Szmek zhangjl02 Zheng Chuan zhenwei pi Zhenyu Zheng Zhenzhong Duan jobs: build-amd64-xsm pass build-arm64-xsm
Re: [PATCH] tools/xenstored: Don't assume errno will not be overwritten in lu_arch()
On 29.07.21 17:23, Julien Grall wrote: On 29/07/2021 12:06, Julien Grall wrote: From: Julien Grall At the moment, do_control_lu() will set errno to 0 before calling lu_arch() and then check errno. The expectation is nothing in lu_arch() will change the value unless there is an error. However, per errno(3), a function that succeeds is allowed to change errno. In fact, syslog() will overwrite errno if the logs are rotated at the time it is called. To prevent any further issue, errno is now always set before returning NULL. Additionally, errno is only checked when returning NULL so the client can see the error message if there is any. Reported-by: Michael Kurth Signed-off-by: Julien Grall --- tools/xenstore/xenstored_control.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c index 6b68b79faac7..6fcb42095b59 100644 --- a/tools/xenstore/xenstored_control.c +++ b/tools/xenstore/xenstored_control.c @@ -324,6 +324,7 @@ static const char *lu_binary_alloc(const void *ctx, struct connection *conn, lu_status->kernel_size = size; lu_status->kernel_off = 0; + errno = 0; return NULL; } @@ -339,6 +340,7 @@ static const char *lu_binary_save(const void *ctx, struct connection *conn, memcpy(lu_status->kernel + lu_status->kernel_off, data, size); lu_status->kernel_off += size; + errno = 0; return NULL; } I forgot to update lu_binary(). I will respin the patch once I get some feedback. With setting errno to 0 before returning NULL in lu_binary() you can add Reviewed-by: Juergen Gross Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
[linux-linus test] 164048: regressions - FAIL
flight 164048 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/164048/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemut-rhel6hvm-intel 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-ws16-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-debianhvm-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-examine 6 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-libvirt 7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemuu-rhel6hvm-amd 7 xen-installfail REGR. vs. 152332 test-amd64-i386-xl-qemut-ws16-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemut-rhel6hvm-amd 7 xen-installfail REGR. vs. 152332 test-amd64-i386-xl7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-debianhvm-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-coresched-i386-xl 7 xen-install fail REGR. vs. 152332 test-amd64-i386-pair 10 xen-install/src_host fail REGR. vs. 152332 test-amd64-i386-pair 11 xen-install/dst_host fail REGR. vs. 152332 test-amd64-i386-libvirt-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-xsm7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemuu-rhel6hvm-intel 7 xen-install fail REGR. vs. 152332 test-amd64-i386-freebsd10-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-pvshim 7 xen-install fail REGR. vs. 152332 test-amd64-i386-freebsd10-i386 7 xen-installfail REGR. vs. 152332 test-amd64-i386-xl-shadow 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-raw7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-ovmf-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-win7-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-win7-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-libvirt-pair 10 xen-install/src_host fail REGR. vs. 152332 test-amd64-i386-libvirt-pair 11 xen-install/dst_host fail REGR. vs. 152332 test-arm64-arm64-xl-thunderx 13 debian-fixup fail REGR. vs. 152332 test-arm64-arm64-xl-xsm 13 debian-fixup fail REGR. vs. 152332 test-arm64-arm64-xl 13 debian-fixup fail REGR. vs. 152332 test-amd64-amd64-amd64-pvgrub 20 guest-stop fail REGR. vs. 152332 test-arm64-arm64-xl-credit1 13 debian-fixup fail REGR. vs. 152332 test-amd64-amd64-i386-pvgrub 20 guest-stop fail REGR. vs. 152332 test-arm64-arm64-xl-credit2 13 debian-fixup fail REGR. vs. 152332 test-arm64-arm64-libvirt-xsm 13 debian-fixup fail REGR. vs. 152332 test-amd64-amd64-xl-xsm 22 guest-start/debian.repeat fail REGR. vs. 152332 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-rtds22 guest-start/debian.repeat fail REGR. vs. 152332 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 152332 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 152332 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 152332 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 152332 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 152332 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 152332 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 152332 test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-arm
Re: [PATCH v1 0/5] PCI: Drop duplicated tracking of a pci_dev's bound driver
On Thu, Jul 29, 2021 at 10:37:35PM +0200, Uwe Kleine-König wrote: > Hello, > > struct pci_dev tracks the bound pci driver twice. This series is about > removing this duplication. > > The first two patches are just cleanups. The third patch introduces a > wrapper that abstracts access to struct pci_dev->driver. In the next > patch (hopefully) all users are converted to use the new wrapper and > finally the fifth patch removes the duplication. > > Note this series is only build tested (allmodconfig on several > architectures). > > I'm open to restructure this series if this simplifies things. E.g. the > use of the new wrapper in drivers/pci could be squashed into the patch > introducing the wrapper. Patch 4 could be split by maintainer tree or > squashed into patch 3 completely. I see only patch 4 and this cover letter... -- With Best Regards, Andy Shevchenko
[PATCH v2 3/3] xen: assume XENFEAT_gnttab_map_avail_bits being set for pv guests
XENFEAT_gnttab_map_avail_bits is always set in Xen 4.0 and newer.
Remove coding assuming it might be zero.
Signed-off-by: Juergen Gross
Acked-by: Peter Zijlstra (Intel)
---
drivers/xen/gntdev.c | 36 ++--
1 file changed, 2 insertions(+), 34 deletions(-)
diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index a3e7be96527d..1e7f6b1c0c97 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -266,20 +266,13 @@ static int find_grant_ptes(pte_t *pte, unsigned long
addr, void *data)
{
struct gntdev_grant_map *map = data;
unsigned int pgnr = (addr - map->vma->vm_start) >> PAGE_SHIFT;
- int flags = map->flags | GNTMAP_application_map | GNTMAP_contains_pte;
+ int flags = map->flags | GNTMAP_application_map | GNTMAP_contains_pte |
+ (1 << _GNTMAP_guest_avail0);
u64 pte_maddr;
BUG_ON(pgnr >= map->count);
pte_maddr = arbitrary_virt_to_machine(pte).maddr;
- /*
-* Set the PTE as special to force get_user_pages_fast() fall
-* back to the slow path. If this is not supported as part of
-* the grant map, it will be done afterwards.
-*/
- if (xen_feature(XENFEAT_gnttab_map_avail_bits))
- flags |= (1 << _GNTMAP_guest_avail0);
-
gnttab_set_map_op(&map->map_ops[pgnr], pte_maddr, flags,
map->grants[pgnr].ref,
map->grants[pgnr].domid);
@@ -288,14 +281,6 @@ static int find_grant_ptes(pte_t *pte, unsigned long addr,
void *data)
return 0;
}
-#ifdef CONFIG_X86
-static int set_grant_ptes_as_special(pte_t *pte, unsigned long addr, void
*data)
-{
- set_pte_at(current->mm, addr, pte, pte_mkspecial(*pte));
- return 0;
-}
-#endif
-
int gntdev_map_grant_pages(struct gntdev_grant_map *map)
{
int i, err = 0;
@@ -1055,23 +1040,6 @@ static int gntdev_mmap(struct file *flip, struct
vm_area_struct *vma)
err = vm_map_pages_zero(vma, map->pages, map->count);
if (err)
goto out_put_map;
- } else {
-#ifdef CONFIG_X86
- /*
-* If the PTEs were not made special by the grant map
-* hypercall, do so here.
-*
-* This is racy since the mapping is already visible
-* to userspace but userspace should be well-behaved
-* enough to not touch it until the mmap() call
-* returns.
-*/
- if (!xen_feature(XENFEAT_gnttab_map_avail_bits)) {
- apply_to_page_range(vma->vm_mm, vma->vm_start,
- vma->vm_end - vma->vm_start,
- set_grant_ptes_as_special, NULL);
- }
-#endif
}
return 0;
--
2.26.2
[PATCH v2 1/3] xen: check required Xen features
Linux kernel is not supported to run on Xen versions older than 4.0.
Add tests for required Xen features always being present in Xen 4.0
and newer.
Signed-off-by: Juergen Gross
---
V2:
- rename macro (Boris Ostrovsky)
- panic() in case of missing feature (Boris Ostrovsky)
---
drivers/xen/features.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/xen/features.c b/drivers/xen/features.c
index 25c053b09605..7b591443833c 100644
--- a/drivers/xen/features.c
+++ b/drivers/xen/features.c
@@ -9,13 +9,26 @@
#include
#include
#include
+#include
#include
+#include
#include
#include
#include
+/*
+ * Linux kernel expects at least Xen 4.0.
+ *
+ * Assume some features to be available for that reason (depending on guest
+ * mode, of course).
+ */
+#define chk_required_feature(f) { \
+ if (!xen_feature(f))\
+ panic("Xen: feature %s not available!\n", #f); \
+ }
+
u8 xen_features[XENFEAT_NR_SUBMAPS * 32] __read_mostly;
EXPORT_SYMBOL_GPL(xen_features);
@@ -31,4 +44,9 @@ void xen_setup_features(void)
for (j = 0; j < 32; j++)
xen_features[i * 32 + j] = !!(fi.submap & 1<
[PATCH v2 0/3] xen: remove some checks for always present Xen features
Some features of Xen can be assumed to be always present, so add a central check to verify this being true and remove the other checks. Juergen Gross (3): xen: check required Xen features xen: assume XENFEAT_mmu_pt_update_preserve_ad being set for pv guests xen: assume XENFEAT_gnttab_map_avail_bits being set for pv guests arch/x86/xen/enlighten_pv.c | 12 ++-- arch/x86/xen/mmu_pv.c | 4 ++-- drivers/xen/features.c | 18 ++ drivers/xen/gntdev.c| 36 ++-- 4 files changed, 24 insertions(+), 46 deletions(-) -- 2.26.2
[PATCH v2 2/3] xen: assume XENFEAT_mmu_pt_update_preserve_ad being set for pv guests
XENFEAT_mmu_pt_update_preserve_ad is always set in Xen 4.0 and newer.
Remove coding assuming it might be zero.
Signed-off-by: Juergen Gross
Acked-by: Peter Zijlstra (Intel)
---
arch/x86/xen/enlighten_pv.c | 12 ++--
arch/x86/xen/mmu_pv.c | 4 ++--
2 files changed, 4 insertions(+), 12 deletions(-)
diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 03149422dce2..753f63734c13 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -116,9 +116,8 @@ static void __init xen_banner(void)
HYPERVISOR_xen_version(XENVER_extraversion, &extra);
pr_info("Booting paravirtualized kernel on %s\n", pv_info.name);
- printk(KERN_INFO "Xen version: %d.%d%s%s\n",
- version >> 16, version & 0x, extra.extraversion,
- xen_feature(XENFEAT_mmu_pt_update_preserve_ad) ? "
(preserve-AD)" : "");
+ pr_info("Xen version: %d.%d%s (preserve-AD)\n",
+ version >> 16, version & 0x, extra.extraversion);
}
static void __init xen_pv_init_platform(void)
@@ -1302,13 +1301,6 @@ asmlinkage __visible void __init xen_start_kernel(void)
xen_init_apic();
#endif
- if (xen_feature(XENFEAT_mmu_pt_update_preserve_ad)) {
- pv_ops.mmu.ptep_modify_prot_start =
- xen_ptep_modify_prot_start;
- pv_ops.mmu.ptep_modify_prot_commit =
- xen_ptep_modify_prot_commit;
- }
-
machine_ops = xen_machine_ops;
/*
diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
index ade789e73ee4..1df5f01529e5 100644
--- a/arch/x86/xen/mmu_pv.c
+++ b/arch/x86/xen/mmu_pv.c
@@ -2099,8 +2099,8 @@ static const struct pv_mmu_ops xen_mmu_ops __initconst = {
.set_pte = xen_set_pte_init,
.set_pmd = xen_set_pmd_hyper,
- .ptep_modify_prot_start = __ptep_modify_prot_start,
- .ptep_modify_prot_commit = __ptep_modify_prot_commit,
+ .ptep_modify_prot_start = xen_ptep_modify_prot_start,
+ .ptep_modify_prot_commit = xen_ptep_modify_prot_commit,
.pte_val = PV_CALLEE_SAVE(xen_pte_val),
.pgd_val = PV_CALLEE_SAVE(xen_pgd_val),
--
2.26.2
Re: [PATCH] tools/xenstored: Propagate correctly the error message from lu_start()
On 29.07.21 13:06, Julien Grall wrote:
From: Julien Grall
lu_start() will only set errno when it returns NULL. For all the
other cases, the value is unknown.
This means that when lu_start() returns an error message, it may not
be propagated to the client.
The check that errno is a non-zero value is now dropped and instead
the value is returned when no error message is provided. This
relies on errno to always be set when ret == NULL.
Fixes: af216a99fb ("tools/xenstore: add the basic framework for doing the live
update")
Signed-off-by: Julien Grall
Reviewed-by: Juergen Gross
Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH] tools/xenstored: Fix off-by-one in dump_state_nodes()
On 29.07.21 11:34, Julien Grall wrote: From: Julien Grall The maximum path length supported by Xenstored protocol is XENSTORE_ABS_PATH_MAX (i.e 3072). This doesn't take into account the NUL at the end of the path. However, the code to dump the nodes will allocate a buffer of XENSTORE_ABS_PATH. As a result it may not be possible to live-update if there is a node name of XENSTORE_ABS_PATH. Fix it by allocating a buffer of XENSTORE_ABS_PATH_MAX + 1 characters. Take the opportunity to pass the max length of the buffer as a parameter of dump_state_node_tree(). This will be clearer that the check in the function is linked to the allocation in dump_state_nodes(). Signed-off-by: Julien Grall Reviewed-by: Juergen Gross --- This was spotted when backporting Live-Update to 4.11 because the commit 924bf8c793 "tools/xenstore: rework path length check" is not present. On the latest upstream, this is looks more a latent bug because I didn't manage to create such large node. Yes, the path length is limited to "/local/domain//" + the max relative path length. Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
