[xen-unstable-smoke test] 150335: tolerable all pass - PUSHED
flight 150335 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/150335/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 5e015d48a5ee68ba03addad2698364d8f015afec baseline version: xen abf378e6483195b98a3f32e2c9d017e0eeeb275f Last test of basis 150333 2020-05-22 21:00:46 Z0 days Testing same since 150335 2020-05-23 01:00:38 Z0 days1 attempts People who touched revisions under test: Nick Rosbrook Nick Rosbrook 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 abf378e648..5e015d48a5 5e015d48a5ee68ba03addad2698364d8f015afec -> smoke
[linux-linus test] 150331: regressions - FAIL
flight 150331 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/150331/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-arm64-arm64-xl-thunderx 7 xen-boot fail REGR. vs. 150281 build-i386-pvops 6 kernel-build fail REGR. vs. 150281 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-debianhvm-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-raw1 build-check(1) blocked n/a test-amd64-i386-examine 1 build-check(1) blocked n/a test-amd64-i386-pair 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-ws16-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-xsm1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-intel 1 build-check(1) blocked n/a test-amd64-i386-xl-shadow 1 build-check(1) blocked n/a test-amd64-i386-qemut-rhel6hvm-intel 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-xl-qemut-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-amd64-i386-qemut-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-coresched-i386-xl 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-win7-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-xl-pvshim 1 build-check(1) blocked n/a test-amd64-i386-xl1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 150281 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 150281 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 150281 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 150281 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 150281 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 150281 test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-check
[xen-unstable test] 150326: regressions - FAIL
flight 150326 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/150326/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-arm64-arm64-xl-seattle 7 xen-boot fail REGR. vs. 150315 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-rtds 16 guest-localmigrate fail REGR. vs. 150315 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 150315 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 150315 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 150315 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 150315 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 150315 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 150315 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 150315 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail like 150315 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 150315 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 150315 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass version targeted for testing: xen f6d102046817bb5c08876ff78a6a00f4d29ee269 baseline version: xen dacdbf7088d6a3705a9831e73991c2b14c519a65 Last test of basis 150315 2020-05-22 01:51:14 Z0 days Testing same since 150326 2020-05-22 14:06:40 Z0 days1 attempts People who touched revisions under test: George Dunlap Wei Liu jobs: build-amd64-xsm
[xen-unstable-smoke test] 150333: tolerable all pass - PUSHED
flight 150333 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/150333/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen abf378e6483195b98a3f32e2c9d017e0eeeb275f baseline version: xen 87827167bb1737e826b0a8fe0abe07c0ace36ac5 Last test of basis 150330 2020-05-22 17:02:53 Z0 days Testing same since 150333 2020-05-22 21:00:46 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Jan Beulich jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 87827167bb..abf378e648 abf378e6483195b98a3f32e2c9d017e0eeeb275f -> smoke
Re: [PATCH 02/10] swiotlb-xen: remove start_dma_addr
On Fri, 22 May 2020, Julien Grall wrote:
> On 22/05/2020 04:55, Stefano Stabellini wrote:
> > On Thu, 21 May 2020, Julien Grall wrote:
> > > Hi,
> > >
> > > On 21/05/2020 00:45, Stefano Stabellini wrote:
> > > > From: Stefano Stabellini
> > > >
> > > > It is not strictly needed. Call virt_to_phys on xen_io_tlb_start
> > > > instead. It will be useful not to have a start_dma_addr around with the
> > > > next patches.
> > > >
> > > > Signed-off-by: Stefano Stabellini
> > > > ---
> > > >drivers/xen/swiotlb-xen.c | 5 +
> > > >1 file changed, 1 insertion(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> > > > index a42129cba36e..b5e0492b07b9 100644
> > > > --- a/drivers/xen/swiotlb-xen.c
> > > > +++ b/drivers/xen/swiotlb-xen.c
> > > > @@ -52,8 +52,6 @@ static unsigned long xen_io_tlb_nslabs;
> > > > * Quick lookup value of the bus address of the IOTLB.
> > > > */
> > > >-static u64 start_dma_addr;
> > > > -
> > > >/*
> > > > * Both of these functions should avoid XEN_PFN_PHYS because
> > > > phys_addr_t
> > > > * can be 32bit when dma_addr_t is 64bit leading to a loss in
> > > > @@ -241,7 +239,6 @@ int __ref xen_swiotlb_init(int verbose, bool early)
> > > > m_ret = XEN_SWIOTLB_EFIXUP;
> > > > goto error;
> > > > }
> > > > - start_dma_addr = xen_virt_to_bus(xen_io_tlb_start);
> > > > if (early) {
> > > > if (swiotlb_init_with_tbl(xen_io_tlb_start,
> > > > xen_io_tlb_nslabs,
> > > > verbose))
> > > > @@ -389,7 +386,7 @@ static dma_addr_t xen_swiotlb_map_page(struct device
> > > > *dev, struct page *page,
> > > > */
> > > > trace_swiotlb_bounced(dev, dev_addr, size, swiotlb_force);
> > > >-map = swiotlb_tbl_map_single(dev, start_dma_addr, phys,
> > > > + map = swiotlb_tbl_map_single(dev,
> > > > virt_to_phys(xen_io_tlb_start),
> > > > phys,
> > >
> > > xen_virt_to_bus() is implemented as xen_phys_to_bus(virt_to_phys()). Can
> > > you
> > > explain how the two are equivalent?
> >
> > They are not equivalent. Looking at what swiotlb_tbl_map_single expects,
> > and also the implementation of swiotlb_init_with_tbl, I think
> > virt_to_phys is actually the one we want.
> >
> > swiotlb_tbl_map_single compares the argument with __pa(tlb) which is
> > __pa(xen_io_tlb_start) which is virt_to_phys(xen_io_tlb_start).
>
> I can't find such check in master. What is your baseline? Could you point to
> the exact line of code?
My base is b85051e755b0e9d6dd8f17ef1da083851b83287d, which is master
from a couple of days back.
xen_swiotlb_init calls swiotlb_init_with_tbl which takes a virt address
as a parameter (xen_io_tlb_start), it gets converted to phys and stored
in io_tlb_start as a physical address.
Later, xen_swiotlb_map_page calls swiotlb_tbl_map_single passing a
dma addr as parameter (tbl_dma_addr). tbl_dma_addr is used to calculate
the right slot in the swiotlb buffer to use. (Strangely tbl_dma_addr is
a dma_addr_t and it is not converted to phys_addr_t before doing
operations... I think tbl_dma_addr should be a phys addr.)
The comparison with io_tlb_start is done here:
do {
while (iommu_is_span_boundary(index, nslots, offset_slots,
max_slots)) {
index += stride;
if (index >= io_tlb_nslabs)
index = 0;
if (index == wrap)
goto not_found;
}
index is io_tlb_start and offset_slots is derived by tbl_dma_addr.
Re: [PATCH 01/10] swiotlb-xen: use vmalloc_to_page on vmalloc virt addresses
On Fri, 22 May 2020, Julien Grall wrote: > Hi Stefano, > > On 22/05/2020 04:54, Stefano Stabellini wrote: > > On Thu, 21 May 2020, Julien Grall wrote: > > > Hi, > > > > > > On 21/05/2020 00:45, Stefano Stabellini wrote: > > > > From: Boris Ostrovsky > > > > > > > > Don't just assume that virt_to_page works on all virtual addresses. > > > > Instead add a is_vmalloc_addr check and use vmalloc_to_page on vmalloc > > > > virt addresses. > > > > > > Can you provide an example where swiotlb is used with vmalloc()? > > > > The issue was reported here happening on the Rasperry Pi 4: > > https://marc.info/?l=xen-devel=158862573216800 > > Thanks, it would be good if the commit message contains a bit more details. > > > > > If you are asking where in the Linux codebase the vmalloc is happening > > specifically, I don't know for sure, my information is limited to the > > stack trace that you see in the link (I don't have a Rasperry Pi 4 yet > > but I shall have one soon.) > > Looking at the code there is a comment in xen_swiotlb_alloc_coherent() > suggesting that xen_alloc_coherent_pages() may return an ioremap'ped region on > Arm. So it feels to me that commit b877ac9815a8fe7e5f6d7fdde3dc34652408840a > "xen/swiotlb: remember having called xen_create_contiguous_region()" has > always been broken on Arm. Yes, I think you are right > As an aside, your commit message also suggests this is an issue for every > virtual address used in swiotlb. But only the virt_to_page() call in > xen_swiotlb_free_coherent() is modified. Is it intended? If yes, I think you > want to update your commit message. I see, yes I can explain better
[xen-unstable-smoke test] 150330: tolerable all pass - PUSHED
flight 150330 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/150330/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 87827167bb1737e826b0a8fe0abe07c0ace36ac5 baseline version: xen 1658a39b0031baf9ec44c8d51d1d1369a964a4f9 Last test of basis 150324 2020-05-22 14:01:40 Z0 days Testing same since 150330 2020-05-22 17:02:53 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Jan Beulich Roger Pau Monné jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 1658a39b00..87827167bb 87827167bb1737e826b0a8fe0abe07c0ace36ac5 -> smoke
[qemu-mainline test] 150320: tolerable FAIL - PUSHED
flight 150320 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/150320/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10 fail like 150312 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 150312 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 150312 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 150312 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 150312 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 150312 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: qemuud19f1ab0de8b763159513e3eaa12c5bc68122361 baseline version: qemuuae3aa5da96f4ccf0c2a28851449d92db9fcfad71 Last test of basis 150312 2020-05-21 23:10:46 Z0 days Testing same since 150320 2020-05-22 11:36:31 Z0 days1 attempts People who touched revisions under test: Amanieu d'Antras Geert Uytterhoeven Guenter Roeck Peter Maydell Philippe Mathieu-Daudé Philippe Mathieu-Daudé Richard Henderson Thomas Huth Wainer dos Santos Moschetta jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64
RE: [PATCH v5 4/5] common/domain: add a domain context record for shared_info...
> -Original Message-
> From: Jan Beulich
> Sent: 22 May 2020 15:34
> To: Paul Durrant
> Cc: xen-devel@lists.xenproject.org; Durrant, Paul ;
> Ian Jackson
> ; Wei Liu ; Andrew Cooper
> ; George
> Dunlap ; Julien Grall ; Stefano
> Stabellini
>
> Subject: RE: [EXTERNAL] [PATCH v5 4/5] common/domain: add a domain context
> record for shared_info...
>
> CAUTION: This email originated from outside of the organization. Do not click
> links or open
> attachments unless you can confirm the sender and know the content is safe.
>
>
>
> On 21.05.2020 18:19, Paul Durrant wrote:
> > @@ -1649,6 +1650,70 @@ int continue_hypercall_on_cpu(
> > return 0;
> > }
> >
> > +static int save_shared_info(const struct domain *d, struct domain_context
> > *c,
> > +bool dry_run)
> > +{
> > +struct domain_shared_info_context ctxt = {
> > +#ifdef CONFIG_COMPAT
> > +.flags = has_32bit_shinfo(d) ? DOMAIN_SAVE_32BIT_SHINFO : 0,
> > +#endif
> > +.buffer_size = sizeof(shared_info_t),
>
> But this size varies between native and compat.
>
> > +static int load_shared_info(struct domain *d, struct domain_context *c)
> > +{
> > +struct domain_shared_info_context ctxt;
> > +size_t hdr_size = offsetof(typeof(ctxt), buffer);
> > +unsigned int i;
> > +int rc;
> > +
> > +rc = DOMAIN_LOAD_BEGIN(SHARED_INFO, c, );
> > +if ( rc )
> > +return rc;
> > +
> > +if ( i ) /* expect only a single instance */
> > +return -ENXIO;
> > +
> > +rc = domain_load_data(c, , hdr_size);
> > +if ( rc )
> > +return rc;
> > +
> > +if ( ctxt.buffer_size != sizeof(shared_info_t) )
> > +return -EINVAL;
>
> While on the save side things could be left as they are (yet
> I'd prefer a change), this should be flexible enough to allow
> at least the smaller compat size as well in the compat case.
Ok, I guess we can optimize the buffer size down if only the compat version is
needed. Seems like slightly pointless complexity though.
> I wonder whether any smaller sizes might be acceptable, once
> again with the rest getting zero-padded.
>
If the need arises to zero extend an older shared_info variant then that can be
done in future.
> > +if ( ctxt.flags & DOMAIN_SAVE_32BIT_SHINFO )
> > +#ifdef CONFIG_COMPAT
> > +has_32bit_shinfo(d) = true;
> > +#else
> > +return -EINVAL;
> > +#endif
>
> Am I mis-remembering or was a check lost of the remaining
> flags being zero? If I am, one needs adding in any case, imo.
>
It wasn't flags before, but you're quite right; they should be zero-checked.
Paul
> Jan
RE: [PATCH v5 1/5] xen/common: introduce a new framework for save/restore of 'domain' context
> -Original Message-
> From: Jan Beulich
> Sent: 22 May 2020 15:25
> To: Paul Durrant
> Cc: xen-devel@lists.xenproject.org; Durrant, Paul ;
> Julien Grall
> ; Andrew Cooper ; George Dunlap
> ;
> Ian Jackson ; Stefano Stabellini
> ; Wei Liu
> ; Volodymyr Babchuk ; Roger Pau
> Monné
> Subject: RE: [EXTERNAL] [PATCH v5 1/5] xen/common: introduce a new framework
> for save/restore of
> 'domain' context
>
> CAUTION: This email originated from outside of the organization. Do not click
> links or open
> attachments unless you can confirm the sender and know the content is safe.
>
>
>
> On 21.05.2020 18:19, Paul Durrant wrote:
> > To allow enlightened HVM guests (i.e. those that have PV drivers) to be
> > migrated without their co-operation it will be necessary to transfer 'PV'
> > state such as event channel state, grant entry state, etc.
> >
> > Currently there is a framework (entered via the hvm_save/load() functions)
> > that allows a domain's 'HVM' (architectural) state to be transferred but
> > 'PV' state is also common with pure PV guests and so this framework is not
> > really suitable.
> >
> > This patch adds the new public header and low level implementation of a new
> > common framework, entered via the domain_save/load() functions. Subsequent
> > patches will introduce other parts of the framework, and code that will
> > make use of it within the current version of the libxc migration stream.
> >
> > This patch also marks the HVM-only framework as deprecated in favour of the
> > new framework.
> >
> > Signed-off-by: Paul Durrant
> > Acked-by: Julien Grall
>
> Reviewed-by: Jan Beulich
Thanks.
> with one remark:
>
> > +int domain_load_end(struct domain_context *c)
> > +{
> > +struct domain *d = c->domain;
> > +size_t len = c->desc.length - c->len;
> > +
> > +while ( c->len != c->desc.length ) /* unconsumed data or pad */
> > +{
> > +uint8_t pad;
> > +int rc = domain_load_data(c, , sizeof(pad));
> > +
> > +if ( rc )
> > +return rc;
> > +
> > +if ( pad )
> > +return -EINVAL;
> > +}
> > +
> > +gdprintk(XENLOG_INFO, "%pd load: %s[%u] +%zu (-%zu)\n", d, c->name,
> > + c->desc.instance, c->len, len);
>
> Unlike on the save side you assume c->name to be non-NULL here.
> We're not going to crash because of this, but it feels a little
> odd anyway, specifically with the function being non-static
> (albeit on the positive side we have the type being private to
> this file).
Yes, I didn't think it was worth the test since it is supposed to be a "can't
happen" case. If we're worried about the load handler calling in with a bad
value of c then we could add a magic number into the struct and ASSERT it is
correct in domain_[save|load]_[begin|data|end].
Paul
>
> Jan
Re: [PATCH 08/10] swiotlb-xen: introduce phys_to_dma/dma_to_phys translations
On 5/22/20 1:34 PM, Stefano Stabellini wrote: > On Thu, 21 May 2020, Boris Ostrovsky wrote: >> On 5/20/20 7:45 PM, Stefano Stabellini wrote: >>> From: Stefano Stabellini >>> >>> Call dma_to_phys in is_xen_swiotlb_buffer. >>> Call phys_to_dma in xen_phys_to_bus. >>> Call dma_to_phys in xen_bus_to_phys. >>> >>> Everything is taken care of by these changes except for >>> xen_swiotlb_alloc_coherent and xen_swiotlb_free_coherent, which need a >>> few explicit phys_to_dma/dma_to_phys calls. >>> >>> Signed-off-by: Stefano Stabellini >>> --- >>> drivers/xen/swiotlb-xen.c | 20 >>> 1 file changed, 12 insertions(+), 8 deletions(-) >>> >>> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c >>> index c50448fd9b75..d011c4c7aa72 100644 >>> --- a/drivers/xen/swiotlb-xen.c >>> +++ b/drivers/xen/swiotlb-xen.c >>> @@ -64,14 +64,16 @@ static inline dma_addr_t xen_phys_to_bus(struct device >>> *dev, phys_addr_t paddr) >>> >>> dma |= paddr & ~XEN_PAGE_MASK; >>> >>> - return dma; >>> + return phys_to_dma(dev, dma); >>> } >>> >>> -static inline phys_addr_t xen_bus_to_phys(struct device *dev, dma_addr_t >>> baddr) >>> +static inline phys_addr_t xen_bus_to_phys(struct device *dev, >>> + dma_addr_t dma_addr) >> >> Since now dma address != bus address this is no longer >> xen_bus_to_phys(). (And I guess the same is rue for xen_phys_to_bus()). > Should I rename them to xen_dma_to_phys and xen_phys_to_dma? Yes, please. -boris
Re: [PATCH 02/10] swiotlb-xen: remove start_dma_addr
Hi,
On 22/05/2020 04:55, Stefano Stabellini wrote:
On Thu, 21 May 2020, Julien Grall wrote:
Hi,
On 21/05/2020 00:45, Stefano Stabellini wrote:
From: Stefano Stabellini
It is not strictly needed. Call virt_to_phys on xen_io_tlb_start
instead. It will be useful not to have a start_dma_addr around with the
next patches.
Signed-off-by: Stefano Stabellini
---
drivers/xen/swiotlb-xen.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index a42129cba36e..b5e0492b07b9 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -52,8 +52,6 @@ static unsigned long xen_io_tlb_nslabs;
* Quick lookup value of the bus address of the IOTLB.
*/
-static u64 start_dma_addr;
-
/*
* Both of these functions should avoid XEN_PFN_PHYS because phys_addr_t
* can be 32bit when dma_addr_t is 64bit leading to a loss in
@@ -241,7 +239,6 @@ int __ref xen_swiotlb_init(int verbose, bool early)
m_ret = XEN_SWIOTLB_EFIXUP;
goto error;
}
- start_dma_addr = xen_virt_to_bus(xen_io_tlb_start);
if (early) {
if (swiotlb_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs,
verbose))
@@ -389,7 +386,7 @@ static dma_addr_t xen_swiotlb_map_page(struct device
*dev, struct page *page,
*/
trace_swiotlb_bounced(dev, dev_addr, size, swiotlb_force);
-map = swiotlb_tbl_map_single(dev, start_dma_addr, phys,
+ map = swiotlb_tbl_map_single(dev, virt_to_phys(xen_io_tlb_start),
phys,
xen_virt_to_bus() is implemented as xen_phys_to_bus(virt_to_phys()). Can you
explain how the two are equivalent?
They are not equivalent. Looking at what swiotlb_tbl_map_single expects,
and also the implementation of swiotlb_init_with_tbl, I think
virt_to_phys is actually the one we want.
swiotlb_tbl_map_single compares the argument with __pa(tlb) which is
__pa(xen_io_tlb_start) which is virt_to_phys(xen_io_tlb_start).
I can't find such check in master. What is your baseline? Could you
point to the exact line of code?
Cheers,
--
Julien Grall
Re: [PATCH 01/10] swiotlb-xen: use vmalloc_to_page on vmalloc virt addresses
Hi Stefano, On 22/05/2020 04:54, Stefano Stabellini wrote: On Thu, 21 May 2020, Julien Grall wrote: Hi, On 21/05/2020 00:45, Stefano Stabellini wrote: From: Boris Ostrovsky Don't just assume that virt_to_page works on all virtual addresses. Instead add a is_vmalloc_addr check and use vmalloc_to_page on vmalloc virt addresses. Can you provide an example where swiotlb is used with vmalloc()? The issue was reported here happening on the Rasperry Pi 4: https://marc.info/?l=xen-devel=158862573216800 Thanks, it would be good if the commit message contains a bit more details. If you are asking where in the Linux codebase the vmalloc is happening specifically, I don't know for sure, my information is limited to the stack trace that you see in the link (I don't have a Rasperry Pi 4 yet but I shall have one soon.) Looking at the code there is a comment in xen_swiotlb_alloc_coherent() suggesting that xen_alloc_coherent_pages() may return an ioremap'ped region on Arm. So it feels to me that commit b877ac9815a8fe7e5f6d7fdde3dc34652408840a "xen/swiotlb: remember having called xen_create_contiguous_region()" has always been broken on Arm. As an aside, your commit message also suggests this is an issue for every virtual address used in swiotlb. But only the virt_to_page() call in xen_swiotlb_free_coherent() is modified. Is it intended? If yes, I think you want to update your commit message. Signed-off-by: Boris Ostrovsky Signed-off-by: Stefano Stabellini --- drivers/xen/swiotlb-xen.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c index b6d27762c6f8..a42129cba36e 100644 --- a/drivers/xen/swiotlb-xen.c +++ b/drivers/xen/swiotlb-xen.c @@ -335,6 +335,7 @@ xen_swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr, int order = get_order(size); phys_addr_t phys; u64 dma_mask = DMA_BIT_MASK(32); + struct page *pg; if (hwdev && hwdev->coherent_dma_mask) dma_mask = hwdev->coherent_dma_mask; @@ -346,9 +347,11 @@ xen_swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr, /* Convert the size to actually allocated. */ size = 1UL << (order + XEN_PAGE_SHIFT); +pg = is_vmalloc_addr(vaddr) ? vmalloc_to_page(vaddr) : + virt_to_page(vaddr); Common DMA code seems to protect this check with CONFIG_DMA_REMAP. Is it something we want to do it here as well? Or is there any other condition where vmalloc can happen? I can see it in dma_direct_free_pages: if (IS_ENABLED(CONFIG_DMA_REMAP) && is_vmalloc_addr(cpu_addr)) vunmap(cpu_addr); I wonder why the common DMA code does that. is_vmalloc_addr should work regardless of CONFIG_DMA_REMAP. Maybe just for efficiency? is_vmalloc_addr() doesn't looks very expensive (although it is not an inline function). So I am not sure the reason behind it. It might be worth asking the author of the config. Cheers, -- Julien Grall
[linux-linus test] 150316: regressions - FAIL
flight 150316 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/150316/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl-vhd 7 xen-boot fail REGR. vs. 150281 test-amd64-amd64-examine 4 memdisk-try-append fail REGR. vs. 150281 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10 fail blocked in 150281 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 150281 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 150281 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 150281 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 150281 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 150281 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 150281 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 150281 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 150281 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass version targeted for testing: linux051143e1602d90ea71887d92363edd539d411de5 baseline version: linux2ea1940b84e55420a9e8feddcafd173edfe4df11 Last test of basis 150281 2020-05-20 19:40:19 Z1 days Failing since150296 2020-05-21 08:13:02 Z1 days3 attempts Testing same since 150316 2020-05-22 03:59:40 Z0 days1 attempts People who touched revisions under test: Andreas Färber Anton Ivanov Christoph Hellwig Christophe
Re: [PATCH 08/10] swiotlb-xen: introduce phys_to_dma/dma_to_phys translations
On Thu, 21 May 2020, Boris Ostrovsky wrote:
> On 5/20/20 7:45 PM, Stefano Stabellini wrote:
> > From: Stefano Stabellini
> >
> > Call dma_to_phys in is_xen_swiotlb_buffer.
> > Call phys_to_dma in xen_phys_to_bus.
> > Call dma_to_phys in xen_bus_to_phys.
> >
> > Everything is taken care of by these changes except for
> > xen_swiotlb_alloc_coherent and xen_swiotlb_free_coherent, which need a
> > few explicit phys_to_dma/dma_to_phys calls.
> >
> > Signed-off-by: Stefano Stabellini
> > ---
> > drivers/xen/swiotlb-xen.c | 20
> > 1 file changed, 12 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> > index c50448fd9b75..d011c4c7aa72 100644
> > --- a/drivers/xen/swiotlb-xen.c
> > +++ b/drivers/xen/swiotlb-xen.c
> > @@ -64,14 +64,16 @@ static inline dma_addr_t xen_phys_to_bus(struct device
> > *dev, phys_addr_t paddr)
> >
> > dma |= paddr & ~XEN_PAGE_MASK;
> >
> > - return dma;
> > + return phys_to_dma(dev, dma);
> > }
> >
> > -static inline phys_addr_t xen_bus_to_phys(struct device *dev, dma_addr_t
> > baddr)
> > +static inline phys_addr_t xen_bus_to_phys(struct device *dev,
> > + dma_addr_t dma_addr)
>
>
> Since now dma address != bus address this is no longer
> xen_bus_to_phys(). (And I guess the same is rue for xen_phys_to_bus()).
Should I rename them to xen_dma_to_phys and xen_phys_to_dma?
> > {
> > + phys_addr_t baddr = dma_to_phys(dev, dma_addr);
> > unsigned long xen_pfn = bfn_to_pfn(XEN_PFN_DOWN(baddr));
> > - dma_addr_t dma = (dma_addr_t)xen_pfn << XEN_PAGE_SHIFT;
> > - phys_addr_t paddr = dma;
> > + phys_addr_t paddr = (xen_pfn << XEN_PAGE_SHIFT) |
> > + (baddr & ~XEN_PAGE_MASK);
> >
> > paddr |= baddr & ~XEN_PAGE_MASK;
>
>
> This line needs to go, no?
Yes, good point
[xen-unstable-smoke test] 150324: tolerable all pass - PUSHED
flight 150324 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/150324/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 1658a39b0031baf9ec44c8d51d1d1369a964a4f9 baseline version: xen f6d102046817bb5c08876ff78a6a00f4d29ee269 Last test of basis 150319 2020-05-22 11:02:07 Z0 days Testing same since 150324 2020-05-22 14:01:40 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Jan Beulich jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git f6d1020468..1658a39b00 1658a39b0031baf9ec44c8d51d1d1369a964a4f9 -> smoke
[PATCH v2 for-4.14 2/2] tools/libxc: xc_memshr_fork with interrupts blocked
Toolstack side for creating forks with interrupt injection blocked.
Signed-off-by: Tamas K Lengyel
Reviewed-by: Roger Pau Monné
Acked-by: Ian Jackson
---
tools/libxc/include/xenctrl.h | 3 ++-
tools/libxc/xc_memshr.c | 4 +++-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 45ff7db1e8..804ff001d7 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2242,7 +2242,8 @@ int xc_memshr_range_share(xc_interface *xch,
int xc_memshr_fork(xc_interface *xch,
uint32_t source_domain,
uint32_t client_domain,
- bool allow_with_iommu);
+ bool allow_with_iommu,
+ bool block_interrupts);
/*
* Note: this function is only intended to be used on short-lived forks that
diff --git a/tools/libxc/xc_memshr.c b/tools/libxc/xc_memshr.c
index 2300cc7075..a6cfd7dccf 100644
--- a/tools/libxc/xc_memshr.c
+++ b/tools/libxc/xc_memshr.c
@@ -240,7 +240,7 @@ int xc_memshr_debug_gref(xc_interface *xch,
}
int xc_memshr_fork(xc_interface *xch, uint32_t pdomid, uint32_t domid,
- bool allow_with_iommu)
+ bool allow_with_iommu, bool block_interrupts)
{
xen_mem_sharing_op_t mso;
@@ -251,6 +251,8 @@ int xc_memshr_fork(xc_interface *xch, uint32_t pdomid,
uint32_t domid,
if ( allow_with_iommu )
mso.u.fork.flags |= XENMEM_FORK_WITH_IOMMU_ALLOWED;
+if ( block_interrupts )
+mso.u.fork.flags |= XENMEM_FORK_BLOCK_INTERRUPTS;
return xc_memshr_memop(xch, domid, );
}
--
2.25.1
[PATCH v2 for-4.14 1/2] x86/mem_sharing: block interrupt injection for forks
When running shallow forks without device models it may be undesirable for Xen
to inject interrupts. With Windows forks we have observed the kernel going into
infinite loops when trying to process such interrupts, likely because it
attempts
to interact with devices that are not responding without QEMU running. By
disabling interrupt injection the fuzzer can exercise the target code without
interference.
Forks & memory sharing are only available on Intel CPUs so this only applies
to vmx.
Signed-off-by: Tamas K Lengyel
---
v2: prohibit => block
minor style adjustments
---
xen/arch/x86/hvm/vmx/intr.c | 6 ++
xen/arch/x86/mm/mem_sharing.c| 6 +-
xen/include/asm-x86/hvm/domain.h | 2 ++
xen/include/public/memory.h | 1 +
4 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/xen/arch/x86/hvm/vmx/intr.c b/xen/arch/x86/hvm/vmx/intr.c
index 000e14af49..80bfbb4787 100644
--- a/xen/arch/x86/hvm/vmx/intr.c
+++ b/xen/arch/x86/hvm/vmx/intr.c
@@ -256,6 +256,12 @@ void vmx_intr_assist(void)
if ( unlikely(v->arch.vm_event) && v->arch.vm_event->sync_event )
return;
+#ifdef CONFIG_MEM_SHARING
+/* Block event injection for VM fork if requested */
+if ( unlikely(v->domain->arch.hvm.mem_sharing.block_interrupts) )
+return;
+#endif
+
/* Crank the handle on interrupt state. */
pt_vector = pt_update_irq(v);
diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index 7271e5c90b..0c45a8d67e 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -2106,7 +2106,8 @@ int
mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
rc = -EINVAL;
if ( mso.u.fork.pad )
goto out;
-if ( mso.u.fork.flags & ~XENMEM_FORK_WITH_IOMMU_ALLOWED )
+if ( mso.u.fork.flags &
+ ~(XENMEM_FORK_WITH_IOMMU_ALLOWED | XENMEM_FORK_BLOCK_INTERRUPTS) )
goto out;
rc = rcu_lock_live_remote_domain_by_id(mso.u.fork.parent_domain,
@@ -2134,6 +2135,9 @@ int
mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
rc = hypercall_create_continuation(__HYPERVISOR_memory_op,
"lh", XENMEM_sharing_op,
arg);
+else if ( !rc && (mso.u.fork.flags & XENMEM_FORK_BLOCK_INTERRUPTS) )
+d->arch.hvm.mem_sharing.block_interrupts = true;
+
rcu_unlock_domain(pd);
break;
}
diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
index 95fe18cddc..37e494d234 100644
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -74,6 +74,8 @@ struct mem_sharing_domain
* to resume the search.
*/
unsigned long next_shared_gfn_to_relinquish;
+
+bool block_interrupts;
};
#endif
diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index dbd35305df..1e4959638d 100644
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -537,6 +537,7 @@ struct xen_mem_sharing_op {
struct mem_sharing_op_fork { /* OP_FORK */
domid_t parent_domain;/* IN: parent's domain id */
#define XENMEM_FORK_WITH_IOMMU_ALLOWED (1u << 0)
+#define XENMEM_FORK_BLOCK_INTERRUPTS (1u << 1)
uint16_t flags; /* IN: optional settings */
uint32_t pad; /* Must be set to 0 */
} fork;
--
2.25.1
[PATCH 3/5] libxl: Generate golang bindings in libxl Makefile
The generated golang bindings (types.gen.go and helpers.gen.go) are left checked in so that they can be fetched from xenbits using the golang tooling. This means that they must be updated whenever libxl_types.idl (or other dependencies) are updated. However, the golang bindings are only built optionally; we can't assume that anyone updating libxl_types.idl will also descend into the tools/golang tree to re-generate the bindings. Fix this by re-generating the golang bindings from the libxl Makefile when the IDL dependencies are updated, so that anyone who updates libxl_types.idl will also end up updating the golang generated files as well. - Make a variable for the generated files, and a target in xenlight/Makefile which will only re-generate the files. - Add a target in libxl/Makefile to call external idl generation targets (currently only golang). For ease of testing, also add a specific target in libxl/Makefile just to check and update files generated from the IDL. This does mean that there are two potential paths for generating the files during a parallel build; but that shouldn't be an issue, since tools/golang/xenlight should never be built until after tools/libxl has completed building anyway. Signed-off-by: George Dunlap --- CC: Ian Jackson CC: Wei Liu CC: Nick Rosbrook --- tools/golang/xenlight/Makefile | 6 +- tools/libxl/Makefile | 12 +++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile index cd0a62505f..751f916276 100644 --- a/tools/golang/xenlight/Makefile +++ b/tools/golang/xenlight/Makefile @@ -17,12 +17,16 @@ all: build .PHONY: package package: $(XEN_GOPATH)$(GOXL_PKG_DIR) -$(XEN_GOPATH)/src/$(XEN_GOCODE_URL)/xenlight/: xenlight.go types.gen.go helpers.gen.go +GOXL_GEN_FILES = types.gen.go helpers.gen.go + +$(XEN_GOPATH)/src/$(XEN_GOCODE_URL)/xenlight/: xenlight.go $(GOXL_GEN_FILES) $(INSTALL_DIR) $(XEN_GOPATH)$(GOXL_PKG_DIR) $(INSTALL_DATA) xenlight.go $(XEN_GOPATH)$(GOXL_PKG_DIR) $(INSTALL_DATA) types.gen.go $(XEN_GOPATH)$(GOXL_PKG_DIR) $(INSTALL_DATA) helpers.gen.go $(XEN_GOPATH)$(GOXL_PKG_DIR) +idl-gen: $(GOXL_GEN_FILES) + %.gen.go: gengotypes.py $(LIBXL_SRC_DIR)/libxl_types.idl $(LIBXL_SRC_DIR)/idl.py XEN_ROOT=$(XEN_ROOT) $(PYTHON) gengotypes.py $(LIBXL_SRC_DIR)/libxl_types.idl diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile index 69fcf21577..2a06a7ebb8 100644 --- a/tools/libxl/Makefile +++ b/tools/libxl/Makefile @@ -218,7 +218,7 @@ testidl.c: libxl_types.idl gentest.py libxl.h $(AUTOINCS) .PHONY: all all: $(CLIENTS) $(TEST_PROGS) $(PKG_CONFIG) $(PKG_CONFIG_LOCAL) \ libxenlight.so libxenlight.a libxlutil.so libxlutil.a \ - $(AUTOSRCS) $(AUTOINCS) + $(AUTOSRCS) $(AUTOINCS) idl-external $(LIBXL_OBJS) $(LIBXLU_OBJS) $(SAVE_HELPER_OBJS) \ $(LIBXL_TEST_OBJS) $(TEST_PROG_OBJS): \ @@ -274,6 +274,16 @@ _libxl_type%.h _libxl_type%_json.h _libxl_type%_private.h _libxl_type%.c: libxl_ $(call move-if-changed,__libxl_type$(stem)_json.h,_libxl_type$(stem)_json.h) $(call move-if-changed,__libxl_type$(stem).c,_libxl_type$(stem).c) +.PHONY: idl-external +idl-external: + $(MAKE) -C $(XEN_ROOT)/tools/golang/xenlight idl-gen + +LIBXL_IDLGEN_FILES = _libxl_types.h _libxl_types_json.h _libxl_types_private.h _libxl_types.c \ + _libxl_types_internal.h _libxl_types_internal_json.h _libxl_types_internal_private.h _libxl_types_internal.c + + +idl-gen: $(LIBXL_GEN_FILES) idl-external + libxenlight.so: libxenlight.so.$(MAJOR) $(SYMLINK_SHLIB) $< $@ -- 2.25.1
[PATCH 5/5] gitignore: Ignore golang package directory
Signed-off-by: George Dunlap --- CC: Ian Jackson CC: Wei Liu CC: Andrew Cooper CC: Jan Beulich CC: Konrad Wilk CC: Stefano Stabellini CC: Julien Grall CC: Nick Rosbrook --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 7418ce9829..c15b49 100644 --- a/.gitignore +++ b/.gitignore @@ -406,6 +406,7 @@ tools/python/xen/lowlevel/xl/_pyxl_types.h tools/xenstore/xenstore-watch tools/xl/_paths.h tools/xl/xl +tools/golang/src docs/txt/misc/*.txt docs/txt/man/*.txt -- 2.25.1
[PATCH 0/5] Golang build fixes
This is a series of patches that improve build for the golang xenlight bindings. The most important patch is #3, which will update the generated golang bindings from the tools/libxl directory when libxl_types.idl is updated, even if the person building doesn't have the golang packages enabled. George Dunlap (5): golang: Add a minimum go version to go.mod golang: Add a variable for the libxl source directory libxl: Generate golang bindings in libxl Makefile golang/xenlight: Use XEN_PKG_DIR variable rather than open-coding gitignore: Ignore golang package directory .gitignore | 1 + tools/golang/xenlight/Makefile | 12 +--- tools/golang/xenlight/go.mod | 2 ++ tools/libxl/Makefile | 12 +++- 4 files changed, 23 insertions(+), 4 deletions(-) -- CC: Ian Jackson CC: Nick Rosbrook CC: Wei Liu
[PATCH 1/5] golang: Add a minimum go version to go.mod
`go build` wants to add the current go version to go.mod as the minimum every time we run `make` in the directory. Add 1.11 (the earliest Go version that supports modules) there to make it happy. Signed-off-by: George Dunlap --- CC: Nick Rosbrook --- tools/golang/xenlight/go.mod | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/golang/xenlight/go.mod b/tools/golang/xenlight/go.mod index 926474d929..7dfbd758d1 100644 --- a/tools/golang/xenlight/go.mod +++ b/tools/golang/xenlight/go.mod @@ -1 +1,3 @@ module xenbits.xenproject.org/git-http/xen.git/tools/golang/xenlight + +go 1.11 -- 2.25.1
[PATCH 4/5] golang/xenlight: Use XEN_PKG_DIR variable rather than open-coding
Signed-off-by: George Dunlap --- CC: Ian Jackson CC: Wei Liu CC: Nick Rosbrook --- tools/golang/xenlight/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile index 751f916276..6ab36c0aa9 100644 --- a/tools/golang/xenlight/Makefile +++ b/tools/golang/xenlight/Makefile @@ -19,7 +19,7 @@ package: $(XEN_GOPATH)$(GOXL_PKG_DIR) GOXL_GEN_FILES = types.gen.go helpers.gen.go -$(XEN_GOPATH)/src/$(XEN_GOCODE_URL)/xenlight/: xenlight.go $(GOXL_GEN_FILES) +$(XEN_GOPATH)$(GOXL_PKG_DIR): xenlight.go $(GOXL_GEN_FILES) $(INSTALL_DIR) $(XEN_GOPATH)$(GOXL_PKG_DIR) $(INSTALL_DATA) xenlight.go $(XEN_GOPATH)$(GOXL_PKG_DIR) $(INSTALL_DATA) types.gen.go $(XEN_GOPATH)$(GOXL_PKG_DIR) -- 2.25.1
[PATCH 2/5] golang: Add a variable for the libxl source directory
...rather than duplicating the path in several places. Signed-off-by: George Dunlap --- CC: Ian Jackson CC: Wei Liu CC: Nick Rosbrook --- tools/golang/xenlight/Makefile | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile index 37ed1358c4..cd0a62505f 100644 --- a/tools/golang/xenlight/Makefile +++ b/tools/golang/xenlight/Makefile @@ -9,6 +9,8 @@ GOXL_INSTALL_DIR = $(GOCODE_DIR)$(GOXL_PKG_DIR) GO ?= go +LIBXL_SRC_DIR = ../../libxl + .PHONY: all all: build @@ -21,8 +23,8 @@ $(XEN_GOPATH)/src/$(XEN_GOCODE_URL)/xenlight/: xenlight.go types.gen.go helpers. $(INSTALL_DATA) types.gen.go $(XEN_GOPATH)$(GOXL_PKG_DIR) $(INSTALL_DATA) helpers.gen.go $(XEN_GOPATH)$(GOXL_PKG_DIR) -%.gen.go: gengotypes.py $(XEN_ROOT)/tools/libxl/libxl_types.idl $(XEN_ROOT)/tools/libxl/idl.py - XEN_ROOT=$(XEN_ROOT) $(PYTHON) gengotypes.py ../../libxl/libxl_types.idl +%.gen.go: gengotypes.py $(LIBXL_SRC_DIR)/libxl_types.idl $(LIBXL_SRC_DIR)/idl.py + XEN_ROOT=$(XEN_ROOT) $(PYTHON) gengotypes.py $(LIBXL_SRC_DIR)/libxl_types.idl # Go will do its own dependency checking, and not actuall go through # with the build if none of the input files have changed. -- 2.25.1
Re: [PATCH] x86/idle: Extend ISR/C6 erratum workaround to Haswell
On 22.05.2020 17:07, Andrew Cooper wrote: > This bug was first discovered against Haswell. It is definitely affected. > > (The XenServer ticket for this bug was opened on 2013-05-30 which is coming up > on 7 years old, and predates Broadwell). > > Signed-off-by: Andrew Cooper Acked-by: Jan Beulich
Re: [PATCH] x86/svm: retry after unhandled NPT fault if gfn was marked for recalculation
On 22/05/2020 14:32, Roger Pau Monné wrote: > On Fri, May 22, 2020 at 02:11:15PM +0100, Andrew Cooper wrote: >> On 22/05/2020 14:04, Jan Beulich wrote: >>> On 22.05.2020 13:11, Roger Pau Monné wrote: That being said, I also don't like the fact that logdity is handled differently between EPT and NPT, as on EPT it's handled as a misconfig while on NPT it's handled as a violation. >>> Because, well, there is no concept of misconfig in NPT. >> Indeed. Intel chose to split EPT errors into two - MISCONFIG for >> structural errors (not present, or reserved bits set) and VIOLATION for >> permissions errors. >> >> AMD reused the same silicon pagewalker design, so have a single >> NPT_FAULT vmexit which behaves much more like a regular pagefault, >> encoding structural vs permission errors in the error code. > Maybe I should clarify, I understand that NPT doesn't have such > differentiation regarding nested page table faults vs EPT, but I feel > like it would be clearer if part of the code could be shared, ie: > unify EPT resolve_misconfig and NPT do_recalc into a single function > for example that uses the necessary p2m-> helpers for the differing > implementations. I think we should be able to tell apart when a NPT > page fault is a recalc one by looking at the bits in the EXITINFO1 > error field? But they use fundamentally different mechanisms. EPT uses an invalid caching type, while NPT sets the User bit (and even this is going to have to change when we want to support GMET for Windows VBS in the long term). You could abstract a few things out into common logic, but none of the bit positions match (not even the recalc software bit), and the result would be more complicated than its current form. ~Andrew
[PATCH] x86/idle: Extend ISR/C6 erratum workaround to Haswell
This bug was first discovered against Haswell. It is definitely affected.
(The XenServer ticket for this bug was opened on 2013-05-30 which is coming up
on 7 years old, and predates Broadwell).
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
We've followed up with Intel, but based on conversations, I was expecting
Haswell to be treated the same as Broadwell in this regard.
---
xen/arch/x86/acpi/cpu_idle.c | 8
1 file changed, 8 insertions(+)
diff --git a/xen/arch/x86/acpi/cpu_idle.c b/xen/arch/x86/acpi/cpu_idle.c
index 178cb607c2..a2248ea11f 100644
--- a/xen/arch/x86/acpi/cpu_idle.c
+++ b/xen/arch/x86/acpi/cpu_idle.c
@@ -583,8 +583,16 @@ bool errata_c6_workaround(void)
* registers), the processor may dispatch the second interrupt (from
* the IRR bit) before the first interrupt has completed and written to
* the EOI register, causing the first interrupt to never complete.
+ *
+ * Note: Haswell hasn't had errata issued, but this issue was first
+ * discovered on Haswell hardware, and is affected.
*/
static const struct x86_cpu_id isr_errata[] = {
+/* Haswell */
+INTEL_FAM6_MODEL(0x3c),
+INTEL_FAM6_MODEL(0x3f),
+INTEL_FAM6_MODEL(0x45),
+INTEL_FAM6_MODEL(0x46),
/* Broadwell */
INTEL_FAM6_MODEL(0x47),
INTEL_FAM6_MODEL(0x3d),
--
2.11.0
[ovmf test] 150318: all pass - PUSHED
flight 150318 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/150318/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 1c877c716038a862e876cac8f0929bab4a96e849 baseline version: ovmf 1a2ad3ba9efdd0db4bf1b6c114eedb59d6c483ca Last test of basis 150313 2020-05-21 23:10:47 Z0 days Testing same since 150318 2020-05-22 05:16:35 Z0 days1 attempts People who touched revisions under test: Liming Gao Michael D Kinney Vitaly Cheptsov jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/osstest/ovmf.git 1a2ad3ba9e..1c877c7160 1c877c716038a862e876cac8f0929bab4a96e849 -> xen-tested-master
Re: [PATCH v5 4/5] common/domain: add a domain context record for shared_info...
On 21.05.2020 18:19, Paul Durrant wrote:
> @@ -1649,6 +1650,70 @@ int continue_hypercall_on_cpu(
> return 0;
> }
>
> +static int save_shared_info(const struct domain *d, struct domain_context *c,
> +bool dry_run)
> +{
> +struct domain_shared_info_context ctxt = {
> +#ifdef CONFIG_COMPAT
> +.flags = has_32bit_shinfo(d) ? DOMAIN_SAVE_32BIT_SHINFO : 0,
> +#endif
> +.buffer_size = sizeof(shared_info_t),
But this size varies between native and compat.
> +static int load_shared_info(struct domain *d, struct domain_context *c)
> +{
> +struct domain_shared_info_context ctxt;
> +size_t hdr_size = offsetof(typeof(ctxt), buffer);
> +unsigned int i;
> +int rc;
> +
> +rc = DOMAIN_LOAD_BEGIN(SHARED_INFO, c, );
> +if ( rc )
> +return rc;
> +
> +if ( i ) /* expect only a single instance */
> +return -ENXIO;
> +
> +rc = domain_load_data(c, , hdr_size);
> +if ( rc )
> +return rc;
> +
> +if ( ctxt.buffer_size != sizeof(shared_info_t) )
> +return -EINVAL;
While on the save side things could be left as they are (yet
I'd prefer a change), this should be flexible enough to allow
at least the smaller compat size as well in the compat case.
I wonder whether any smaller sizes might be acceptable, once
again with the rest getting zero-padded.
> +if ( ctxt.flags & DOMAIN_SAVE_32BIT_SHINFO )
> +#ifdef CONFIG_COMPAT
> +has_32bit_shinfo(d) = true;
> +#else
> +return -EINVAL;
> +#endif
Am I mis-remembering or was a check lost of the remaining
flags being zero? If I am, one needs adding in any case, imo.
Jan
Re: [PATCH v5 1/5] xen/common: introduce a new framework for save/restore of 'domain' context
On 21.05.2020 18:19, Paul Durrant wrote:
> To allow enlightened HVM guests (i.e. those that have PV drivers) to be
> migrated without their co-operation it will be necessary to transfer 'PV'
> state such as event channel state, grant entry state, etc.
>
> Currently there is a framework (entered via the hvm_save/load() functions)
> that allows a domain's 'HVM' (architectural) state to be transferred but
> 'PV' state is also common with pure PV guests and so this framework is not
> really suitable.
>
> This patch adds the new public header and low level implementation of a new
> common framework, entered via the domain_save/load() functions. Subsequent
> patches will introduce other parts of the framework, and code that will
> make use of it within the current version of the libxc migration stream.
>
> This patch also marks the HVM-only framework as deprecated in favour of the
> new framework.
>
> Signed-off-by: Paul Durrant
> Acked-by: Julien Grall
Reviewed-by: Jan Beulich
with one remark:
> +int domain_load_end(struct domain_context *c)
> +{
> +struct domain *d = c->domain;
> +size_t len = c->desc.length - c->len;
> +
> +while ( c->len != c->desc.length ) /* unconsumed data or pad */
> +{
> +uint8_t pad;
> +int rc = domain_load_data(c, , sizeof(pad));
> +
> +if ( rc )
> +return rc;
> +
> +if ( pad )
> +return -EINVAL;
> +}
> +
> +gdprintk(XENLOG_INFO, "%pd load: %s[%u] +%zu (-%zu)\n", d, c->name,
> + c->desc.instance, c->len, len);
Unlike on the save side you assume c->name to be non-NULL here.
We're not going to crash because of this, but it feels a little
odd anyway, specifically with the function being non-static
(albeit on the positive side we have the type being private to
this file).
Jan
Re: [PATCH v3 1/3] x86: relax GDT check in arch_set_info_guest()
On 22.05.2020 15:27, Andrew Cooper wrote: > On 20/05/2020 08:53, Jan Beulich wrote: >> It is wrong for us to check frames beyond the guest specified limit >> (in the compat case another loop bound is already correct). >> >> Signed-off-by: Jan Beulich > > I'm still not overly convinced this is a good idea, because all it will > allow people to do is write lazy code which breaks on older Xen. Sounds a little like keeping bugs for the sake of keeping things broken. The range of misbehaving versions could be shrunk by backporting this change; I didn't intend to though, so far. > However, if you still insist, Acked-by: Andrew Cooper > Thanks! Jan
Re: [PATCH v4] x86/idle: prevent entering C6 with in service interrupts on Intel
On 22.05.2020 15:54, Roger Pau Monné wrote: > On Fri, May 22, 2020 at 03:42:10PM +0200, Jan Beulich wrote: >> On 21.05.2020 11:22, Roger Pau Monne wrote: >>> Apply a workaround for Intel errata BDX99, CLX30, SKX100, CFW125, >>> BDF104, BDH85, BDM135, KWB131: "A Pending Fixed Interrupt May Be >>> Dispatched Before an Interrupt of The Same Priority Completes". >> >> While the change looks good to me as far as Broadwell goes, I >> think it was before this posting that Andrew also pointed at >> a specific Haswell erratum instance, still on the v3 thread. >> Am I to imply a v5 will follow adding affected Haswell models >> to the table? > > Those refer to a different errata, see: > > https://lore.kernel.org/xen-devel/84486d84-4452-af18-f7e7-753faf5a1...@citrix.com/ > > So we believe this also affects Haswell, but the errata is not published > for those CPUs (yet at least). If/when it's published I'm happy to add > it, in the meantime I think we should go with what has been published > by Intel. Oh, sorry, yes - should have looked at the full thread again, not just Andrew's initial response. Reviewed-by: Jan Beulich Jan
Re: [PATCH v2] x86/ioemul: Rewrite stub generation to be shadow stack compatible
On 22.05.2020 12:24, Andrew Cooper wrote:
> --- a/xen/arch/x86/ioport_emulate.c
> +++ b/xen/arch/x86/ioport_emulate.c
> @@ -8,7 +8,10 @@
> #include
> #include
>
> -static bool ioemul_handle_proliant_quirk(
> +unsigned int __read_mostly (*ioemul_handle_quirk)(
I guess the more correct (and long term supported) placement is
unsigned int (*__read_mostly ioemul_handle_quirk)(
as you mean to modify the variable, not its type.
> --- a/xen/arch/x86/pv/emul-priv-op.c
> +++ b/xen/arch/x86/pv/emul-priv-op.c
> @@ -47,51 +47,97 @@ struct priv_op_ctxt {
> unsigned int bpmatch;
> };
>
> -/* I/O emulation support. Helper routines for, and type of, the stack stub.
> */
> -void host_to_guest_gpr_switch(struct cpu_user_regs *);
> -unsigned long guest_to_host_gpr_switch(unsigned long);
> +/* I/O emulation helpers. Use non-standard calling conventions. */
> +void nocall load_guest_gprs(struct cpu_user_regs *);
> +void nocall save_guest_gprs(void);
>
> typedef void io_emul_stub_t(struct cpu_user_regs *);
>
> static io_emul_stub_t *io_emul_stub_setup(struct priv_op_ctxt *ctxt, u8
> opcode,
>unsigned int port, unsigned int
> bytes)
> {
> +/*
> + * Construct a stub for IN/OUT emulation.
> + *
> + * Some platform drivers communicate with the SMM handler using GPRs as a
> + * mailbox. Therefore, we must perform the emulation with the hardware
> + * domain's registers in view.
> + *
> + * We write a stub of the following form, using the guest load/save
> + * helpers (non-standard ABI), and one of several possible stubs
> + * performing the real I/O.
> + */
> +static const char prologue[] = {
> +0x53, /* push %rbx */
> +0x55, /* push %rbp */
> +0x41, 0x54, /* push %r12 */
> +0x41, 0x55, /* push %r13 */
> +0x41, 0x56, /* push %r14 */
> +0x41, 0x57, /* push %r15 */
> +0x57, /* push %rdi (param for save_guest_gprs) */
> +}; /* call load_guest_gprs */
> +/* */
> +/* call save_guest_gprs */
> +static const char epilogue[] = {
> +0x5f, /* pop %rdi */
> +0x41, 0x5f, /* pop %r15 */
> +0x41, 0x5e, /* pop %r14 */
> +0x41, 0x5d, /* pop %r13 */
> +0x41, 0x5c, /* pop %r12 */
> +0x5d, /* pop %rbp */
> +0x5b, /* pop %rbx */
> +0xc3, /* ret */
> +};
> +
> struct stubs *this_stubs = _cpu(stubs);
> unsigned long stub_va = this_stubs->addr + STUB_BUF_SIZE / 2;
> -long disp;
> -bool use_quirk_stub = false;
> +unsigned int quirk_bytes = 0;
> +char *p;
> +
> +/* Helpers - Read outer scope but only modify p. */
> +#define APPEND_BUFF(b) ({ memcpy(p, b, sizeof(b)); p += sizeof(b); })
> +#define APPEND_CALL(f) \
> +({ \
> +long disp = (long)(f) - (stub_va + p - ctxt->io_emul_stub + 5); \
> +BUG_ON((int32_t)disp != disp); \
> +*p++ = 0xe8;\
> +*(int32_t *)p = disp; p += 4; \
> +})
>
> if ( !ctxt->io_emul_stub )
> ctxt->io_emul_stub =
> map_domain_page(_mfn(this_stubs->mfn)) + (stub_va & ~PAGE_MASK);
>
> -/* call host_to_guest_gpr_switch */
> -ctxt->io_emul_stub[0] = 0xe8;
> -disp = (long)host_to_guest_gpr_switch - (stub_va + 5);
> -BUG_ON((int32_t)disp != disp);
> -*(int32_t *)>io_emul_stub[1] = disp;
> +p = ctxt->io_emul_stub;
> +
> +APPEND_BUFF(prologue);
> +APPEND_CALL(load_guest_gprs);
>
> +/* Some platforms might need to quirk the stub for specific inputs. */
> if ( unlikely(ioemul_handle_quirk) )
> -use_quirk_stub = ioemul_handle_quirk(opcode, >io_emul_stub[5],
> - ctxt->ctxt.regs);
> +{
> +quirk_bytes = ioemul_handle_quirk(opcode, p, ctxt->ctxt.regs);
> +p += quirk_bytes;
> +}
>
> -if ( !use_quirk_stub )
> +/* Default I/O stub. */
> +if ( likely(!quirk_bytes) )
> {
> -/* data16 or nop */
> -ctxt->io_emul_stub[5] = (bytes != 2) ? 0x90 : 0x66;
> -/* */
> -ctxt->io_emul_stub[6] = opcode;
> -/* imm8 or nop */
> -ctxt->io_emul_stub[7] = !(opcode & 8) ? port : 0x90;
> -/* ret (jumps to guest_to_host_gpr_switch) */
> -ctxt->io_emul_stub[8] = 0xc3;
> +*p++ = (bytes != 2) ? 0x90 : 0x66; /* data16 or nop */
> +*p++ = opcode; /* */
> +*p++ = !(opcode & 8) ? port : 0x90; /* imm8 or nop */
> }
>
> -BUILD_BUG_ON(STUB_BUF_SIZE / 2 < MAX(9, /* Default emul stub */
> - 5 +
[xen-unstable-smoke test] 150319: tolerable all pass - PUSHED
flight 150319 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/150319/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen f6d102046817bb5c08876ff78a6a00f4d29ee269 baseline version: xen dacdbf7088d6a3705a9831e73991c2b14c519a65 Last test of basis 150280 2020-05-20 18:01:10 Z1 days Testing same since 150319 2020-05-22 11:02:07 Z0 days1 attempts People who touched revisions under test: George Dunlap Wei Liu 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 dacdbf7088..f6d1020468 f6d102046817bb5c08876ff78a6a00f4d29ee269 -> smoke
Re: [PATCH v4] x86/idle: prevent entering C6 with in service interrupts on Intel
On Fri, May 22, 2020 at 03:42:10PM +0200, Jan Beulich wrote: > On 21.05.2020 11:22, Roger Pau Monne wrote: > > Apply a workaround for Intel errata BDX99, CLX30, SKX100, CFW125, > > BDF104, BDH85, BDM135, KWB131: "A Pending Fixed Interrupt May Be > > Dispatched Before an Interrupt of The Same Priority Completes". > > While the change looks good to me as far as Broadwell goes, I > think it was before this posting that Andrew also pointed at > a specific Haswell erratum instance, still on the v3 thread. > Am I to imply a v5 will follow adding affected Haswell models > to the table? Those refer to a different errata, see: https://lore.kernel.org/xen-devel/84486d84-4452-af18-f7e7-753faf5a1...@citrix.com/ So we believe this also affects Haswell, but the errata is not published for those CPUs (yet at least). If/when it's published I'm happy to add it, in the meantime I think we should go with what has been published by Intel. Roger.
Re: [PATCH v2] x86/traps: Rework #PF[Rsvd] bit handling
On 21.05.2020 17:43, Andrew Cooper wrote:
> @@ -1439,6 +1418,21 @@ void do_page_fault(struct cpu_user_regs *regs)
> if ( unlikely(fixup_page_fault(addr, regs) != 0) )
> return;
>
> +/*
> + * Xen doesn't have reserved bits set in its pagetables, nor do we permit
> + * PV guests to write any. Such entries would generally be vulnerable to
> + * the L1TF sidechannel.
> + *
> + * The shadow pagetable logic may use reserved bits as part of
> + * SHOPT_FAST_FAULT_PATH. Pagefaults arising from these will be resolved
> + * via the fixup_page_fault() path.
> + *
> + * Anything remaining is an error, constituting corruption of the
> + * pagetables and probably an L1TF vulnerable gadget.
> + */
> +if ( error_code & PFEC_reserved_bit )
> +goto fatal;
> +
> if ( unlikely(!guest_mode(regs)) )
> {
> enum pf_type pf_type = spurious_page_fault(addr, regs);
> @@ -1457,13 +1451,12 @@ void do_page_fault(struct cpu_user_regs *regs)
> if ( likely((fixup = search_exception_table(regs)) != 0) )
While I continue to not fully agree with not trying to fix up such
faults if the fault location has recovery code attached, I realize
that we're not going to reach agreement here, so somewhat hesitantly
Acked-by: Jan Beulich
Jan
Re: [PATCH v4] x86/idle: prevent entering C6 with in service interrupts on Intel
On 21.05.2020 11:22, Roger Pau Monne wrote: > Apply a workaround for Intel errata BDX99, CLX30, SKX100, CFW125, > BDF104, BDH85, BDM135, KWB131: "A Pending Fixed Interrupt May Be > Dispatched Before an Interrupt of The Same Priority Completes". While the change looks good to me as far as Broadwell goes, I think it was before this posting that Andrew also pointed at a specific Haswell erratum instance, still on the v3 thread. Am I to imply a v5 will follow adding affected Haswell models to the table? Jan
Re: [PATCH v7 00/19] Add support for qemu-xen runnning in a Linux-based stubdomain
Jason Andryuk writes ("Re: [PATCH v7 00/19] Add support for qemu-xen runnning
in a Linux-based stubdomain"):
> I can do this. What is the SUPPORT status for this?
I think that given we aren't testing it upstream, the answer
probably has to be "Tech Preview".
In general, I would love to see this (including your stub builder)
being tested by osstest.
Ian.
Re: [PATCH] x86/svm: retry after unhandled NPT fault if gfn was marked for recalculation
On 22.05.2020 12:19, Andrew Cooper wrote: > On 22/05/2020 11:05, Igor Druzhinin wrote: >> On 22/05/2020 10:45, Andrew Cooper wrote: >>> On 21/05/2020 22:43, Igor Druzhinin wrote: If a recalculation NPT fault hasn't been handled explicitly in hvm_hap_nested_page_fault() then it's potentially safe to retry - US bit has been re-instated in PTE and any real fault would be correctly re-raised next time. This covers a specific case of migration with vGPU assigned on AMD: global log-dirty is enabled and causes immediate recalculation NPT fault in MMIO area upon access. This type of fault isn't described explicitly in hvm_hap_nested_page_fault (this isn't called on EPT misconfig exit on Intel) which results in domain crash. Signed-off-by: Igor Druzhinin --- xen/arch/x86/hvm/svm/svm.c | 4 1 file changed, 4 insertions(+) diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index 46a1aac..f0d0bd3 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -1726,6 +1726,10 @@ static void svm_do_nested_pgfault(struct vcpu *v, /* inject #VMEXIT(NPF) into guest. */ nestedsvm_vmexit_defer(v, VMEXIT_NPF, pfec, gpa); return; +case 0: +/* If a recalculation page fault hasn't been handled - just retry. */ +if ( pfec & PFEC_user_mode ) +return; >>> This smells like it is a recipe for livelocks. >>> >>> Everything should have been handled properly by the call to >>> p2m_pt_handle_deferred_changes() which precedes svm_do_nested_pgfault(). >>> >>> It is legitimate for the MMIO mapping to end up being transiently >>> recalculated, but the fact that p2m_pt_handle_deferred_changes() doesn't >>> fix it up suggests that the bug is there. >>> >>> Do you have the complete NPT walk to the bad mapping? Do we have >>> _PAGE_USER in the leaf mapping, or is this perhaps a spurious fault? >> It does fix it up. The problem is that currently in SVM we enter >> svm_do_nested_pgfault immediately after p2m_pt_handle_deferred_changes >> is finished finished. > > Oh - so we do. I'd read the entry condition for svm_do_nested_pgfault() > incorrectly. > > Jan - why did you chose to do it this way? If > p2m_pt_handle_deferred_changes() has made a modification, there is > surely nothing relevant to do in svm_do_nested_pgfault(). Why? There can very well be multiple reasons for a single exit to occur, and hence it did seem to make sense to allow processing of such possible further reasons right away, instead of re- entering the guest just to deal with another VM exit. What I can accept is that the error code may not correctly represent the "remaining" fault reason anymore at this point. Jan
Re: [PATCH] x86/svm: retry after unhandled NPT fault if gfn was marked for recalculation
On Fri, May 22, 2020 at 02:11:15PM +0100, Andrew Cooper wrote: > On 22/05/2020 14:04, Jan Beulich wrote: > > On 22.05.2020 13:11, Roger Pau Monné wrote: > >> That being said, I also don't like the fact that logdity is handled > >> differently between EPT and NPT, as on EPT it's handled as a > >> misconfig while on NPT it's handled as a violation. > > Because, well, there is no concept of misconfig in NPT. > > Indeed. Intel chose to split EPT errors into two - MISCONFIG for > structural errors (not present, or reserved bits set) and VIOLATION for > permissions errors. > > AMD reused the same silicon pagewalker design, so have a single > NPT_FAULT vmexit which behaves much more like a regular pagefault, > encoding structural vs permission errors in the error code. Maybe I should clarify, I understand that NPT doesn't have such differentiation regarding nested page table faults vs EPT, but I feel like it would be clearer if part of the code could be shared, ie: unify EPT resolve_misconfig and NPT do_recalc into a single function for example that uses the necessary p2m-> helpers for the differing implementations. I think we should be able to tell apart when a NPT page fault is a recalc one by looking at the bits in the EXITINFO1 error field? Anyway, this was just a rant, and it's tangential to the issue at hand, sorry for distracting. Roger.
Re: [PATCH v7 00/19] Add support for qemu-xen runnning in a Linux-based stubdomain
On Fri, May 22, 2020 at 5:54 AM Paul Durrant wrote: > > > -Original Message- > > From: Xen-devel On Behalf Of > > George Dunlap > > Sent: 22 May 2020 10:11 > > To: Jason Andryuk > > Cc: Stefano Stabellini ; Julien Grall > > ; Samuel Thibault > > ; Wei Liu ; Andrew Cooper > > ; Jan > > Beulich ; Ian Jackson ; Anthony > > Perard > > ; xen-devel ; > > Daniel De Graaf > > > > Subject: Re: [PATCH v7 00/19] Add support for qemu-xen runnning in a > > Linux-based stubdomain > > > > > > > On May 19, 2020, at 2:54 AM, Jason Andryuk wrote: > > > > > > General idea is to allow freely set device_model_version and > > > device_model_stubdomain_override and choose the right options based on > > > this > > > choice. Also, allow to specific path to stubdomain kernel/ramdisk, for > > > greater > > > flexibility. > > > > Excited to see this patch series get in. But I didn’t really notice any > > documents explaining how to > > actually use it — is there a blog post anywhere describing how to get the > > kernel / initrd image and so > > on? Yeah, it's not really collected anywhere, but below are the quick start instructions. The cover letter mentioned this repo (forked from Marek's): https://github.com/jandryuk/qubes-vmm-xen-stubdom-linux (branch initramfs-tools, tag for-upstream-v6) clone it and then run: $ make get-sources $ make -f Makefile.stubdom output: kernel: build/linux/arch/x86/boot/bzImage ramdisk: build/rootfs/stubdom-linux-rootfs To make them available system wide, copy to /usr/lib/xen/boot/qemu-stubdom-linux-kernel and /usr/lib/xen/boot/qemu-stubdom-linux-rootfs respectively. Obviously this should match your installation's "$lib/xen/boot/" location. A second option is to set paths to those files manually in a VM's xl.cfg with stubdomain_kernel="/path" and stubdomain_ramdisk="/path" Update your xl configuration with: device_model_stubdomain_override = 1 device_model_version = "qemu-xen" Start the domain and that should be it. Maybe additionally use serial = "pty" to access the VM with `xl console -t serial $NAME`. Some limitations are here: https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=docs/misc/stubdom.txt;h=c717a95d17d2e562639a5574e89df3c4db8712fa;hb=HEAD#l124 Limitations: - PCI passthrough require permissive mode - only one nic is supported - at most 26 emulated disks are supported (more are still available as PV disks) - graphics output (VNC/SDL/Spice) not supported > > Also, would it be possible to add a follow-up series which modifies > > SUPPORT.md and CHANGELOG.md? > > Yes please. In future I think we should encourage the patch to CHANGELOG.md > to be the last patch of a series such as this. I can do this. What is the SUPPORT status for this? Regards, Jason
Re: [PATCH v3 1/3] x86: relax GDT check in arch_set_info_guest()
On 20/05/2020 08:53, Jan Beulich wrote: > It is wrong for us to check frames beyond the guest specified limit > (in the compat case another loop bound is already correct). > > Signed-off-by: Jan Beulich I'm still not overly convinced this is a good idea, because all it will allow people to do is write lazy code which breaks on older Xen. However, if you still insist, Acked-by: Andrew Cooper
Re: [PATCH] x86/svm: retry after unhandled NPT fault if gfn was marked for recalculation
On 22/05/2020 14:04, Jan Beulich wrote: > On 22.05.2020 13:11, Roger Pau Monné wrote: >> That being said, I also don't like the fact that logdity is handled >> differently between EPT and NPT, as on EPT it's handled as a >> misconfig while on NPT it's handled as a violation. > Because, well, there is no concept of misconfig in NPT. Indeed. Intel chose to split EPT errors into two - MISCONFIG for structural errors (not present, or reserved bits set) and VIOLATION for permissions errors. AMD reused the same silicon pagewalker design, so have a single NPT_FAULT vmexit which behaves much more like a regular pagefault, encoding structural vs permission errors in the error code. ~Andrew
Re: [PATCH] m4: use test instead of []
Wei Liu writes ("[PATCH] m4: use test instead of []"):
> It is reported that [] was removed by autoconf, which caused the
> following error:
>
> ./configure: line 4681: -z: command not found
>
> Switch to test. That's what is used throughout our configure scripts.
The reason for [ ] being removed is that configure.ac et al are
processed by m4 with quote characters set to [ ].
> APPEND_LDFLAGS="$APPEND_LDFLAGS -L$ldflag"
> done
> -if [ ! -z $EXTRA_PREFIX ]; then
> +if test ! -z $EXTRA_PREFIX ; then
> CPPFLAGS="$CPPFLAGS -I$EXTRA_PREFIX/include"
If $EXTRA_PREFIX contains nothing (or just whitespace) this expands to
test ! -z
which only works by accident. It is parsed ax
if not (string_is_nonempty("-z"))
Variable expansions in test expressions should generally be in " ".
Ian.
Re: [PATCH] x86/svm: retry after unhandled NPT fault if gfn was marked for recalculation
On 22.05.2020 13:11, Roger Pau Monné wrote: > That being said, I also don't like the fact that logdity is handled > differently between EPT and NPT, as on EPT it's handled as a > misconfig while on NPT it's handled as a violation. Because, well, there is no concept of misconfig in NPT. Jan
Re: [PATCH for-4.14 2/2] tools/libxc: xc_memshr_fork with interrupts disabled
Tamas K Lengyel writes ("[PATCH for-4.14 2/2] tools/libxc: xc_memshr_fork with
interrupts disabled"):
> Toolstack side for creating forks with interrupt injection disabled.
Acked-by: Ian Jackson
Subject to the hypervisor folks being happy with the underlying
feature.
Ian.
Re: [PATCH] xen/trace: Don't dump offline CPUs in debugtrace_dump_worker()
On 21.05.2020 10:44, Andrew Cooper wrote: > The 'T' debugkey reliably wedges on one of my systems, which has a sparse > APIC_ID layout due to a non power-of-2 number of cores per socket. The > per_cpu(dt_cpu_data, cpu) calcution falls over the deliberately non-canonical > poison value. > > Signed-off-by: Andrew Cooper Acked-by: Jan Beulich
[PATCH v5 39/38] drm: xen: fix common struct sg_table related issues
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
Fix the code to refer to proper nents or orig_nents entries. This driver
reports the number of the pages in the imported scatterlist, so it should
refer to sg_table->orig_nents entry.
Signed-off-by: Marek Szyprowski
Acked-by: Oleksandr Andrushchenko
---
For more information, see '[PATCH v5 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/linux-iommu/20200513132114.6046-1-m.szyprow...@samsung.com/T/
This patch has been resurrected on Oleksandr Andrushchenko request. The
patch was a part of v2 patchset, but then I've dropped it as it only
fixes the debug output, thus I didn't consider it so critical.
---
drivers/gpu/drm/xen/xen_drm_front_gem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c
b/drivers/gpu/drm/xen/xen_drm_front_gem.c
index f0b85e0..ba4bdc5 100644
--- a/drivers/gpu/drm/xen/xen_drm_front_gem.c
+++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c
@@ -215,7 +215,7 @@ struct drm_gem_object *
return ERR_PTR(ret);
DRM_DEBUG("Imported buffer of size %zu with nents %u\n",
- size, sgt->nents);
+ size, sgt->orig_nents);
return _obj->base;
}
--
1.9.1
[xen-unstable test] 150315: tolerable FAIL
flight 150315 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/150315/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10 fail like 150285 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 150285 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 150285 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 150285 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 150285 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 150285 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 150285 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 150285 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail like 150285 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 150285 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 150285 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass version targeted for testing: xen dacdbf7088d6a3705a9831e73991c2b14c519a65 baseline version: xen dacdbf7088d6a3705a9831e73991c2b14c519a65 Last test of basis 150315 2020-05-22 01:51:14 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
Xen 4.13.1 released
All, I am pleased to announce the release of Xen 4.13.1. This is available immediately from its git repository http://xenbits.xen.org/gitweb/?p=xen.git;a=shortlog;h=refs/heads/stable-4.13 (tag RELEASE-4.13.1) or from the XenProject download page https://xenproject.org/downloads/xen-project-archives/xen-project-4-13-series/xen-project-4-13-1/ (where a list of changes can also be found). We recommend all users of the 4.13 stable series to update to this first point release. Regards, Jan
Xen 4.12.3 released
All, I am pleased to announce the release of Xen 4.12.3. This is available immediately from its git repository http://xenbits.xen.org/gitweb/?p=xen.git;a=shortlog;h=refs/heads/stable-4.12 (tag RELEASE-4.12.3) or from the XenProject download page https://xenproject.org/downloads/xen-project-archives/xen-project-4-12-series/xen-project-4-12-3/ (where a list of changes can also be found). We recommend all users of the 4.12 stable series to update to this latest point release. Regards, Jan
Re: [PATCH] x86: refine guest_mode()
On 22.05.2020 12:48, Roger Pau Monné wrote: > On Fri, May 22, 2020 at 11:52:42AM +0200, Jan Beulich wrote: >> On 20.05.2020 17:13, Roger Pau Monné wrote: >>> OK, so I think I'm starting to understand this all. Sorry it's taken >>> me so long. So it's my understanding that diff != 0 can only happen in >>> Xen context, or when in an IST that has a different stack (ie: MCE, NMI >>> or DF according to current.h) and running in PV mode? >>> >>> Wouldn't in then be fine to use (r)->cs & 3 to check we are in guest >>> mode if diff != 0? I see a lot of other places where cs & 3 is already >>> used to that effect AFAICT (like entry.S). >> >> Technically this would be correct afaics, but the idea with all this >> is (or should I say "looks to be"?) to have the checks be as tight as >> possible, to make sure we don't mistakenly consider something "guest >> mode" which really isn't. IOW your suggestion would be fine with me >> if we could exclude bugs anywhere in the code. But since this isn't >> realistic, I consider your suggestion to be relaxing things by too >> much. > > OK, so I take that (long time) we might also want to change the cs & 3 > checks from entry.S to check against __HYPERVISOR_CS explicitly? I didn't think so, no (not the least because of there not being any guarantee afaik that EFI runtime calls couldn't play with segment registers; they shouldn't, yes, but there's a lot of other "should" many don't obey to). Those are guaranteed PV-only code paths. The main issue here is that ->cs cannot be relied upon when a frame points at HVM state. Jan
Re: [PATCH v3 1/5] x86: suppress XPTI-related TLB flushes when possible
On 22/05/2020 12:13, Roger Pau Monné wrote: > On Fri, May 22, 2020 at 12:00:14PM +0100, Andrew Cooper wrote: >> On 25/09/2019 16:23, Jan Beulich wrote: >>> When there's no XPTI-enabled PV domain at all, there's no need to issue >>> respective TLB flushes. Hardwire opt_xpti_* to false when !PV, and >>> record the creation of PV domains by bumping opt_xpti_* accordingly. >>> >>> As to the sticky opt_xpti_domu vs increment/decrement of opt_xpti_hwdom, >>> this is done this way to avoid >>> (a) widening the former variable, >>> (b) any risk of a missed flush, which would result in an XSA if a DomU >>> was able to exercise it, and >>> (c) any races updating the variable. >>> Fundamentally the TLB flush done when context switching out the domain's >>> vCPU-s the last time before destroying the domain ought to be >>> sufficient, so in principle DomU handling could be made match hwdom's. >>> >>> Signed-off-by: Jan Beulich >> I am still concerned about the added complexity for no obvious use case. >> >> Under what circumstances do we expect to XPTI-ness come and go on a >> system, outside of custom dev-testing scenarios? > XPTI-ness will be sticky, in the sense that once enabled cannot be > disabled anymore. I guess the question was a little too rhetorical, so lets spell it out. You're either on Meltdown vulnerable hardware, or not. If not, none of this logic is relevant (AFAICT). If you're on Meltdown-vulnerable hardware and in a production environment, your running with XPTI (at which point none of this complexity is relevant). The only plausible case I can see where this would make a difference is a dev environment starting with a non-XPTI dom0, then booting an XPTI guest, which which point can we seriously justify bizarre logic like: opt_xpti_hwdom -= IS_ENABLED(CONFIG_LATE_HWDOM) && !d->domain_id && opt_xpti_hwdom; just for an alleged perf improvement in a development corner case? ~Andrew
Re: [PATCH v3 1/5] x86: suppress XPTI-related TLB flushes when possible
On 22.05.2020 13:00, Andrew Cooper wrote: > On 25/09/2019 16:23, Jan Beulich wrote: >> When there's no XPTI-enabled PV domain at all, there's no need to issue >> respective TLB flushes. Hardwire opt_xpti_* to false when !PV, and >> record the creation of PV domains by bumping opt_xpti_* accordingly. >> >> As to the sticky opt_xpti_domu vs increment/decrement of opt_xpti_hwdom, >> this is done this way to avoid >> (a) widening the former variable, >> (b) any risk of a missed flush, which would result in an XSA if a DomU >> was able to exercise it, and >> (c) any races updating the variable. >> Fundamentally the TLB flush done when context switching out the domain's >> vCPU-s the last time before destroying the domain ought to be >> sufficient, so in principle DomU handling could be made match hwdom's. >> >> Signed-off-by: Jan Beulich > > I am still concerned about the added complexity for no obvious use case. > > Under what circumstances do we expect to XPTI-ness come and go on a > system, outside of custom dev-testing scenarios? Run a PVH Dom0 with just HVM guests for a while on a system, until you find a need to run a PV guest there (perhaps because of an emergency). Jan
Re: [PATCH 2/3] configure: also add EXTRA_PREFIX to {CPP/LD}FLAGS
> On 22 May 2020, at 12:19, Roger Pau Monné wrote: > > On Fri, May 22, 2020 at 10:05:53AM +0100, Wei Liu wrote: >> On Fri, May 22, 2020 at 08:41:17AM +, Bertrand Marquis wrote: >>> Hi, >>> >>> As a consequence of this fix, the following has been committed (I guess as >>> a consequence of regenerating the configure scripts): >>> diff --git a/tools/configure b/tools/configure >>> index 375430df3f..36596389b8 100755 >>> --- a/tools/configure >>> +++ b/tools/configure >>> @@ -4678,6 +4678,10 @@ for ldflag in $APPEND_LIB >>> do >>> APPEND_LDFLAGS="$APPEND_LDFLAGS -L$ldflag" >>> done >>> +if ! -z $EXTRA_PREFIX ; then >>> +CPPFLAGS="$CPPFLAGS -I$EXTRA_PREFIX/include" >>> +LDFLAGS="$LDFLAGS -L$EXTRA_PREFIX/lib" >>> +fi >>> CPPFLAGS="$PREPEND_CPPFLAGS $CPPFLAGS $APPEND_CPPFLAGS" >>> LDFLAGS="$PREPEND_LDFLAGS $LDFLAGS $APPEND_LDFLAGS” >>> >>> This should be: >>> if [ ! -z $EXTRA_PREFIX ]; then >>> >>> As on other configure scripts. >>> >>> During configure I have not the following error: >>> ./configure: line 4681: -z: command not found >>> >>> Which is ignored but is adding -L/lib and -I/include to the CPPFLAGS and >>> LDFLAGS >>> >>> What should be the procedure to actually fix that (as the problem is coming >>> from the configure script regeneration I guess) ? >> >> Does the following patch work for you? >> >> diff --git a/m4/set_cflags_ldflags.m4 b/m4/set_cflags_ldflags.m4 >> index 08f5c983cc63..cd34c139bc94 100644 >> --- a/m4/set_cflags_ldflags.m4 >> +++ b/m4/set_cflags_ldflags.m4 >> @@ -15,7 +15,7 @@ for ldflag in $APPEND_LIB >> do >> APPEND_LDFLAGS="$APPEND_LDFLAGS -L$ldflag" >> done >> -if [ ! -z $EXTRA_PREFIX ]; then >> +if test ! -z $EXTRA_PREFIX ; then >> CPPFLAGS="$CPPFLAGS -I$EXTRA_PREFIX/include" >> LDFLAGS="$LDFLAGS -L$EXTRA_PREFIX/lib" >> fi > > Reviewed-by: Roger Pau Monné Reviewed-by: Bertrand Marquis > > My bad, I assume [] is expanded by m4, as that seems to be part of the > language? > > Thanks, Roger.
Re: [PATCH 2/3] configure: also add EXTRA_PREFIX to {CPP/LD}FLAGS
On Fri, May 22, 2020 at 11:40:06AM +, Bertrand Marquis wrote: > > > > On 22 May 2020, at 12:19, Roger Pau Monné wrote: > > > > On Fri, May 22, 2020 at 10:05:53AM +0100, Wei Liu wrote: > >> On Fri, May 22, 2020 at 08:41:17AM +, Bertrand Marquis wrote: > >>> Hi, > >>> > >>> As a consequence of this fix, the following has been committed (I guess > >>> as a consequence of regenerating the configure scripts): > >>> diff --git a/tools/configure b/tools/configure > >>> index 375430df3f..36596389b8 100755 > >>> --- a/tools/configure > >>> +++ b/tools/configure > >>> @@ -4678,6 +4678,10 @@ for ldflag in $APPEND_LIB > >>> do > >>> APPEND_LDFLAGS="$APPEND_LDFLAGS -L$ldflag" > >>> done > >>> +if ! -z $EXTRA_PREFIX ; then > >>> +CPPFLAGS="$CPPFLAGS -I$EXTRA_PREFIX/include" > >>> +LDFLAGS="$LDFLAGS -L$EXTRA_PREFIX/lib" > >>> +fi > >>> CPPFLAGS="$PREPEND_CPPFLAGS $CPPFLAGS $APPEND_CPPFLAGS" > >>> LDFLAGS="$PREPEND_LDFLAGS $LDFLAGS $APPEND_LDFLAGS” > >>> > >>> This should be: > >>> if [ ! -z $EXTRA_PREFIX ]; then > >>> > >>> As on other configure scripts. > >>> > >>> During configure I have not the following error: > >>> ./configure: line 4681: -z: command not found > >>> > >>> Which is ignored but is adding -L/lib and -I/include to the CPPFLAGS and > >>> LDFLAGS > >>> > >>> What should be the procedure to actually fix that (as the problem is > >>> coming from the configure script regeneration I guess) ? > >> > >> Does the following patch work for you? > >> > >> diff --git a/m4/set_cflags_ldflags.m4 b/m4/set_cflags_ldflags.m4 > >> index 08f5c983cc63..cd34c139bc94 100644 > >> --- a/m4/set_cflags_ldflags.m4 > >> +++ b/m4/set_cflags_ldflags.m4 > >> @@ -15,7 +15,7 @@ for ldflag in $APPEND_LIB > >> do > >> APPEND_LDFLAGS="$APPEND_LDFLAGS -L$ldflag" > >> done > >> -if [ ! -z $EXTRA_PREFIX ]; then > >> +if test ! -z $EXTRA_PREFIX ; then > >> CPPFLAGS="$CPPFLAGS -I$EXTRA_PREFIX/include" > >> LDFLAGS="$LDFLAGS -L$EXTRA_PREFIX/lib" > >> fi > > > > Reviewed-by: Roger Pau Monné > Reviewed-by: Bertrand Marquis Thanks. I will transfer your tag to the proper patch I just sent. Wei.
Re: [PATCH v3 2/5] x86/mm: honor opt_pcid also for 32-bit PV domains
On 25/09/2019 16:23, Jan Beulich wrote: > I can't see any technical or performance reason why we should treat > 32-bit PV different from 64-bit PV in this regard. > > Signed-off-by: Jan Beulich > Reviewed-by: Roger Pau Monné There are technical reasons, and a very good perf reason not to... There is no such thing as a user/kernel split for 32bit guests (as TF_kernel_mode remains set unconditionally), and there is no such thing as an XPTI split (32bit code can't attack Xen using meltdown). What you would gain is the perf hit of maintaining unused PCID's worth of mappings (seeing as INVPCID is horribly expensive even on modern CPUs). The only way this might not hurt performance is if it was tied to a PV ABI extension letting 32bit PV guests split their user/kernel mappings and have Xen handle the transition automatically, at which point a user/kernel PCID split in Xen would be better than the guest kernel trying to do KPTI on its own. ~Andrew
[PATCH] m4: use test instead of []
It is reported that [] was removed by autoconf, which caused the
following error:
./configure: line 4681: -z: command not found
Switch to test. That's what is used throughout our configure scripts.
Reported-by: Bertrand Marquis
Fixes: 8a6b1665d987 ("configure: also add EXTRA_PREFIX to {CPP/LD}FLAGS")
Reviewed-by: Roger Pau Monne
Signed-off-by: Wei Liu
---
Run autogen.sh before committing.
Cc: Bertrand Marquis
Cc: Roger Pau Monne
---
m4/set_cflags_ldflags.m4 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/m4/set_cflags_ldflags.m4 b/m4/set_cflags_ldflags.m4
index 08f5c983cc63..cd34c139bc94 100644
--- a/m4/set_cflags_ldflags.m4
+++ b/m4/set_cflags_ldflags.m4
@@ -15,7 +15,7 @@ for ldflag in $APPEND_LIB
do
APPEND_LDFLAGS="$APPEND_LDFLAGS -L$ldflag"
done
-if [ ! -z $EXTRA_PREFIX ]; then
+if test ! -z $EXTRA_PREFIX ; then
CPPFLAGS="$CPPFLAGS -I$EXTRA_PREFIX/include"
LDFLAGS="$LDFLAGS -L$EXTRA_PREFIX/lib"
fi
--
2.20.1
Re: [PATCH 2/3] configure: also add EXTRA_PREFIX to {CPP/LD}FLAGS
On Fri, May 22, 2020 at 10:05:53AM +0100, Wei Liu wrote: > On Fri, May 22, 2020 at 08:41:17AM +, Bertrand Marquis wrote: > > Hi, > > > > As a consequence of this fix, the following has been committed (I guess as > > a consequence of regenerating the configure scripts): > > diff --git a/tools/configure b/tools/configure > > index 375430df3f..36596389b8 100755 > > --- a/tools/configure > > +++ b/tools/configure > > @@ -4678,6 +4678,10 @@ for ldflag in $APPEND_LIB > > do > > APPEND_LDFLAGS="$APPEND_LDFLAGS -L$ldflag" > > done > > +if ! -z $EXTRA_PREFIX ; then > > +CPPFLAGS="$CPPFLAGS -I$EXTRA_PREFIX/include" > > +LDFLAGS="$LDFLAGS -L$EXTRA_PREFIX/lib" > > +fi > > CPPFLAGS="$PREPEND_CPPFLAGS $CPPFLAGS $APPEND_CPPFLAGS" > > LDFLAGS="$PREPEND_LDFLAGS $LDFLAGS $APPEND_LDFLAGS” > > > > This should be: > > if [ ! -z $EXTRA_PREFIX ]; then > > > > As on other configure scripts. > > > > During configure I have not the following error: > > ./configure: line 4681: -z: command not found > > > > Which is ignored but is adding -L/lib and -I/include to the CPPFLAGS and > > LDFLAGS > > > > What should be the procedure to actually fix that (as the problem is coming > > from the configure script regeneration I guess) ? > > Does the following patch work for you? > > diff --git a/m4/set_cflags_ldflags.m4 b/m4/set_cflags_ldflags.m4 > index 08f5c983cc63..cd34c139bc94 100644 > --- a/m4/set_cflags_ldflags.m4 > +++ b/m4/set_cflags_ldflags.m4 > @@ -15,7 +15,7 @@ for ldflag in $APPEND_LIB > do > APPEND_LDFLAGS="$APPEND_LDFLAGS -L$ldflag" > done > -if [ ! -z $EXTRA_PREFIX ]; then > +if test ! -z $EXTRA_PREFIX ; then > CPPFLAGS="$CPPFLAGS -I$EXTRA_PREFIX/include" > LDFLAGS="$LDFLAGS -L$EXTRA_PREFIX/lib" > fi Reviewed-by: Roger Pau Monné My bad, I assume [] is expanded by m4, as that seems to be part of the language? Thanks, Roger.
Re: [PATCH v3 1/5] x86: suppress XPTI-related TLB flushes when possible
On Fri, May 22, 2020 at 12:00:14PM +0100, Andrew Cooper wrote: > On 25/09/2019 16:23, Jan Beulich wrote: > > When there's no XPTI-enabled PV domain at all, there's no need to issue > > respective TLB flushes. Hardwire opt_xpti_* to false when !PV, and > > record the creation of PV domains by bumping opt_xpti_* accordingly. > > > > As to the sticky opt_xpti_domu vs increment/decrement of opt_xpti_hwdom, > > this is done this way to avoid > > (a) widening the former variable, > > (b) any risk of a missed flush, which would result in an XSA if a DomU > > was able to exercise it, and > > (c) any races updating the variable. > > Fundamentally the TLB flush done when context switching out the domain's > > vCPU-s the last time before destroying the domain ought to be > > sufficient, so in principle DomU handling could be made match hwdom's. > > > > Signed-off-by: Jan Beulich > > I am still concerned about the added complexity for no obvious use case. > > Under what circumstances do we expect to XPTI-ness come and go on a > system, outside of custom dev-testing scenarios? XPTI-ness will be sticky, in the sense that once enabled cannot be disabled anymore. Roger.
Re: [PATCH] x86/svm: retry after unhandled NPT fault if gfn was marked for recalculation
On Fri, May 22, 2020 at 11:27:38AM +0100, Igor Druzhinin wrote: > On 22/05/2020 11:23, Roger Pau Monné wrote: > > On Fri, May 22, 2020 at 11:14:24AM +0100, Igor Druzhinin wrote: > >> On 22/05/2020 11:08, Roger Pau Monné wrote: > >>> On Thu, May 21, 2020 at 10:43:58PM +0100, Igor Druzhinin wrote: > If a recalculation NPT fault hasn't been handled explicitly in > hvm_hap_nested_page_fault() then it's potentially safe to retry - > US bit has been re-instated in PTE and any real fault would be correctly > re-raised next time. > > This covers a specific case of migration with vGPU assigned on AMD: > global log-dirty is enabled and causes immediate recalculation NPT > fault in MMIO area upon access. This type of fault isn't described > explicitly in hvm_hap_nested_page_fault (this isn't called on > EPT misconfig exit on Intel) which results in domain crash. > >>> > >>> Couldn't direct MMIO regions be handled like other types of memory for > >>> the purposes of logdiry mode? > >>> > >>> I assume there's already a path here used for other memory types when > >>> logdirty is turned on, and hence would seem better to just make direct > >>> MMIO regions also use that path? > >> > >> The proble of handling only MMIO case is that the issue still stays. > >> It will be hit with some other memory type since it's not MMIO specific. > >> The issue is that if global recalculation is called, the next hit to > >> this type will cause a transient fault which will not be handled > >> correctly after a due fixup by neither of our handlers. > > > > I admit I should go look at the code, but for example RAM p2m types > > don't require this fix, so I assume there's some different path taken > > in that case that avoids all this? > > > > Ie: when global logdirty is enabled you will start to get nested page > > faults for every access, yet only direct MMIO types require this fix? > > It's not "only MMIO" - it's just MMIO area is hit in my particular case. > I'd prefer this fix to address the general issue otherwise for SVM > we would have to write handlers in hvm_hap_nested_page_fault() for > every case as soon as we hit it. Hm, I'm not sure I agree. p2m memory types are limited, and IMO we want to have strict control about how they are handled. hvm_hap_nested_page_fault is already full of special casing for each memory type for that reason. That being said, I also don't like the fact that logdity is handled differently between EPT and NPT, as on EPT it's handled as a misconfig while on NPT it's handled as a violation. Thanks, Roger.
Re: [PATCH v3 1/5] x86: suppress XPTI-related TLB flushes when possible
On 25/09/2019 16:23, Jan Beulich wrote: > When there's no XPTI-enabled PV domain at all, there's no need to issue > respective TLB flushes. Hardwire opt_xpti_* to false when !PV, and > record the creation of PV domains by bumping opt_xpti_* accordingly. > > As to the sticky opt_xpti_domu vs increment/decrement of opt_xpti_hwdom, > this is done this way to avoid > (a) widening the former variable, > (b) any risk of a missed flush, which would result in an XSA if a DomU > was able to exercise it, and > (c) any races updating the variable. > Fundamentally the TLB flush done when context switching out the domain's > vCPU-s the last time before destroying the domain ought to be > sufficient, so in principle DomU handling could be made match hwdom's. > > Signed-off-by: Jan Beulich I am still concerned about the added complexity for no obvious use case. Under what circumstances do we expect to XPTI-ness come and go on a system, outside of custom dev-testing scenarios?
Re: [PATCH] x86: refine guest_mode()
On Fri, May 22, 2020 at 11:52:42AM +0200, Jan Beulich wrote: > On 20.05.2020 17:13, Roger Pau Monné wrote: > > OK, so I think I'm starting to understand this all. Sorry it's taken > > me so long. So it's my understanding that diff != 0 can only happen in > > Xen context, or when in an IST that has a different stack (ie: MCE, NMI > > or DF according to current.h) and running in PV mode? > > > > Wouldn't in then be fine to use (r)->cs & 3 to check we are in guest > > mode if diff != 0? I see a lot of other places where cs & 3 is already > > used to that effect AFAICT (like entry.S). > > Technically this would be correct afaics, but the idea with all this > is (or should I say "looks to be"?) to have the checks be as tight as > possible, to make sure we don't mistakenly consider something "guest > mode" which really isn't. IOW your suggestion would be fine with me > if we could exclude bugs anywhere in the code. But since this isn't > realistic, I consider your suggestion to be relaxing things by too > much. OK, so I take that (long time) we might also want to change the cs & 3 checks from entry.S to check against __HYPERVISOR_CS explicitly? What I would prefer is to have some kind of homogeneity in how guest mode vs Xen mode checks are performed, so that we don't confuse people. Thanks, Roger.
Re: [PATCH v3 3/5] x86/HVM: move NOFLUSH handling out of hvm_set_cr3()
On 25/09/2019 16:25, Jan Beulich wrote: > The bit is meaningful only for MOV-to-CR3 insns, not anywhere else, in > particular not when loading nested guest state. > > Signed-off-by: Jan Beulich > Reviewed-by: Paul Durrant Acked-by: Andrew Cooper
Re: [PATCH] golang: Update generated files after libxl_types.idl change
> On May 22, 2020, at 11:01 AM, Wei Liu wrote:
>
> On Fri, May 22, 2020 at 10:49:56AM +0100, George Dunlap wrote:
>> c/s 7efd9f3d45 ("libxl: Handle Linux stubdomain specific QEMU
>> options.") modified libl_types.idl. Run gengotypes.py again to update
>
> libxl_types.idl.
>
>> the geneated golang bindings.
>>
>
> Can we perhaps add a dependency on golang's side such that it can be
> auto-generated in the future?
Indeed, I’m trying to think of a good solution to this.
-George
Re: [PATCH] x86/svm: retry after unhandled NPT fault if gfn was marked for recalculation
On 22/05/2020 11:23, Roger Pau Monné wrote: > On Fri, May 22, 2020 at 11:14:24AM +0100, Igor Druzhinin wrote: >> On 22/05/2020 11:08, Roger Pau Monné wrote: >>> On Thu, May 21, 2020 at 10:43:58PM +0100, Igor Druzhinin wrote: If a recalculation NPT fault hasn't been handled explicitly in hvm_hap_nested_page_fault() then it's potentially safe to retry - US bit has been re-instated in PTE and any real fault would be correctly re-raised next time. This covers a specific case of migration with vGPU assigned on AMD: global log-dirty is enabled and causes immediate recalculation NPT fault in MMIO area upon access. This type of fault isn't described explicitly in hvm_hap_nested_page_fault (this isn't called on EPT misconfig exit on Intel) which results in domain crash. >>> >>> Couldn't direct MMIO regions be handled like other types of memory for >>> the purposes of logdiry mode? >>> >>> I assume there's already a path here used for other memory types when >>> logdirty is turned on, and hence would seem better to just make direct >>> MMIO regions also use that path? >> >> The proble of handling only MMIO case is that the issue still stays. >> It will be hit with some other memory type since it's not MMIO specific. >> The issue is that if global recalculation is called, the next hit to >> this type will cause a transient fault which will not be handled >> correctly after a due fixup by neither of our handlers. > > I admit I should go look at the code, but for example RAM p2m types > don't require this fix, so I assume there's some different path taken > in that case that avoids all this? > > Ie: when global logdirty is enabled you will start to get nested page > faults for every access, yet only direct MMIO types require this fix? It's not "only MMIO" - it's just MMIO area is hit in my particular case. I'd prefer this fix to address the general issue otherwise for SVM we would have to write handlers in hvm_hap_nested_page_fault() for every case as soon as we hit it. Igor
[PATCH v2] x86/ioemul: Rewrite stub generation to be shadow stack compatible
The logic is completely undocumented and almost impossible to follow. It
actually uses return oriented programming. Rewrite it to conform to more
normal call mechanics, and leave a big comment explaining thing. As well as
the code being easier to follow, it will execute faster as it isn't fighting
the branch predictor.
Move the ioemul_handle_quirk() function pointer from traps.c to
ioport_emulate.c. There is no reason for it to be in neither of the two
translation units which use it. Alter the behaviour to return the number of
bytes written into the stub.
Introduce a new nocall annotation using __attribute__((error)) to prohibit
calls being made.
Signed-off-by: Andrew Cooper
--
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
v2:
* Add ELF metadata for {load,save}_guest_gprs()
* Make ioemul_handle_quirk() __read_mostly
* Add new nocall tag
---
xen/arch/x86/ioport_emulate.c | 11 ++---
xen/arch/x86/pv/emul-priv-op.c | 92 +++---
xen/arch/x86/pv/gpr_switch.S | 44
xen/arch/x86/traps.c | 3 --
xen/include/asm-x86/io.h | 3 +-
xen/include/xen/compiler.h | 2 +
6 files changed, 95 insertions(+), 60 deletions(-)
diff --git a/xen/arch/x86/ioport_emulate.c b/xen/arch/x86/ioport_emulate.c
index 499c1f6056..b9d5d27188 100644
--- a/xen/arch/x86/ioport_emulate.c
+++ b/xen/arch/x86/ioport_emulate.c
@@ -8,7 +8,10 @@
#include
#include
-static bool ioemul_handle_proliant_quirk(
+unsigned int __read_mostly (*ioemul_handle_quirk)(
+uint8_t opcode, char *io_emul_stub, struct cpu_user_regs *regs);
+
+static unsigned int ioemul_handle_proliant_quirk(
u8 opcode, char *io_emul_stub, struct cpu_user_regs *regs)
{
static const char stub[] = {
@@ -19,18 +22,16 @@ static bool ioemul_handle_proliant_quirk(
0xa8, 0x80, /*test $0x80, %al */
0x75, 0xfb, /*jnz 1b */
0x9d, /*popf*/
-0xc3, /*ret */
};
uint16_t port = regs->dx;
uint8_t value = regs->al;
if ( (opcode != 0xee) || (port != 0xcd4) || !(value & 0x80) )
-return false;
+return 0;
memcpy(io_emul_stub, stub, sizeof(stub));
-BUILD_BUG_ON(IOEMUL_QUIRK_STUB_BYTES < sizeof(stub));
-return true;
+return sizeof(stub);
}
/* This table is the set of system-specific I/O emulation hooks. */
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index 3b705299cf..3fb36a3e19 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -47,51 +47,97 @@ struct priv_op_ctxt {
unsigned int bpmatch;
};
-/* I/O emulation support. Helper routines for, and type of, the stack stub. */
-void host_to_guest_gpr_switch(struct cpu_user_regs *);
-unsigned long guest_to_host_gpr_switch(unsigned long);
+/* I/O emulation helpers. Use non-standard calling conventions. */
+void nocall load_guest_gprs(struct cpu_user_regs *);
+void nocall save_guest_gprs(void);
typedef void io_emul_stub_t(struct cpu_user_regs *);
static io_emul_stub_t *io_emul_stub_setup(struct priv_op_ctxt *ctxt, u8 opcode,
unsigned int port, unsigned int
bytes)
{
+/*
+ * Construct a stub for IN/OUT emulation.
+ *
+ * Some platform drivers communicate with the SMM handler using GPRs as a
+ * mailbox. Therefore, we must perform the emulation with the hardware
+ * domain's registers in view.
+ *
+ * We write a stub of the following form, using the guest load/save
+ * helpers (non-standard ABI), and one of several possible stubs
+ * performing the real I/O.
+ */
+static const char prologue[] = {
+0x53, /* push %rbx */
+0x55, /* push %rbp */
+0x41, 0x54, /* push %r12 */
+0x41, 0x55, /* push %r13 */
+0x41, 0x56, /* push %r14 */
+0x41, 0x57, /* push %r15 */
+0x57, /* push %rdi (param for save_guest_gprs) */
+}; /* call load_guest_gprs */
+/* */
+/* call save_guest_gprs */
+static const char epilogue[] = {
+0x5f, /* pop %rdi */
+0x41, 0x5f, /* pop %r15 */
+0x41, 0x5e, /* pop %r14 */
+0x41, 0x5d, /* pop %r13 */
+0x41, 0x5c, /* pop %r12 */
+0x5d, /* pop %rbp */
+0x5b, /* pop %rbx */
+0xc3, /* ret */
+};
+
struct stubs *this_stubs = _cpu(stubs);
unsigned long stub_va = this_stubs->addr + STUB_BUF_SIZE / 2;
-long disp;
-bool use_quirk_stub = false;
+unsigned int quirk_bytes = 0;
+char *p;
+
+/* Helpers - Read outer scope but only modify p. */
+#define APPEND_BUFF(b) ({ memcpy(p, b, sizeof(b)); p += sizeof(b); })
+#define APPEND_CALL(f) \
+({ \
+
Re: [PATCH] x86/svm: retry after unhandled NPT fault if gfn was marked for recalculation
On 22/05/2020 11:19, Andrew Cooper wrote: > On 22/05/2020 11:05, Igor Druzhinin wrote: >> On 22/05/2020 10:45, Andrew Cooper wrote: >>> On 21/05/2020 22:43, Igor Druzhinin wrote: If a recalculation NPT fault hasn't been handled explicitly in hvm_hap_nested_page_fault() then it's potentially safe to retry - US bit has been re-instated in PTE and any real fault would be correctly re-raised next time. This covers a specific case of migration with vGPU assigned on AMD: global log-dirty is enabled and causes immediate recalculation NPT fault in MMIO area upon access. This type of fault isn't described explicitly in hvm_hap_nested_page_fault (this isn't called on EPT misconfig exit on Intel) which results in domain crash. Signed-off-by: Igor Druzhinin --- xen/arch/x86/hvm/svm/svm.c | 4 1 file changed, 4 insertions(+) diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index 46a1aac..f0d0bd3 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -1726,6 +1726,10 @@ static void svm_do_nested_pgfault(struct vcpu *v, /* inject #VMEXIT(NPF) into guest. */ nestedsvm_vmexit_defer(v, VMEXIT_NPF, pfec, gpa); return; +case 0: +/* If a recalculation page fault hasn't been handled - just retry. */ +if ( pfec & PFEC_user_mode ) +return; >>> This smells like it is a recipe for livelocks. >>> >>> Everything should have been handled properly by the call to >>> p2m_pt_handle_deferred_changes() which precedes svm_do_nested_pgfault(). >>> >>> It is legitimate for the MMIO mapping to end up being transiently >>> recalculated, but the fact that p2m_pt_handle_deferred_changes() doesn't >>> fix it up suggests that the bug is there. >>> >>> Do you have the complete NPT walk to the bad mapping? Do we have >>> _PAGE_USER in the leaf mapping, or is this perhaps a spurious fault? >> It does fix it up. The problem is that currently in SVM we enter >> svm_do_nested_pgfault immediately after p2m_pt_handle_deferred_changes >> is finished finished. > > Oh - so we do. I'd read the entry condition for svm_do_nested_pgfault() > incorrectly. > > Jan - why did you chose to do it this way? If > p2m_pt_handle_deferred_changes() has made a modification, there is > surely nothing relevant to do in svm_do_nested_pgfault(). In Jan's defense that saves one additional VMEXIT in rare cases if the fault had other implications (write to RO page in log-dirty) in addition to recalculation. Igor other
Re: [PATCH] x86/svm: retry after unhandled NPT fault if gfn was marked for recalculation
On Fri, May 22, 2020 at 11:14:24AM +0100, Igor Druzhinin wrote: > On 22/05/2020 11:08, Roger Pau Monné wrote: > > On Thu, May 21, 2020 at 10:43:58PM +0100, Igor Druzhinin wrote: > >> If a recalculation NPT fault hasn't been handled explicitly in > >> hvm_hap_nested_page_fault() then it's potentially safe to retry - > >> US bit has been re-instated in PTE and any real fault would be correctly > >> re-raised next time. > >> > >> This covers a specific case of migration with vGPU assigned on AMD: > >> global log-dirty is enabled and causes immediate recalculation NPT > >> fault in MMIO area upon access. This type of fault isn't described > >> explicitly in hvm_hap_nested_page_fault (this isn't called on > >> EPT misconfig exit on Intel) which results in domain crash. > > > > Couldn't direct MMIO regions be handled like other types of memory for > > the purposes of logdiry mode? > > > > I assume there's already a path here used for other memory types when > > logdirty is turned on, and hence would seem better to just make direct > > MMIO regions also use that path? > > The proble of handling only MMIO case is that the issue still stays. > It will be hit with some other memory type since it's not MMIO specific. > The issue is that if global recalculation is called, the next hit to > this type will cause a transient fault which will not be handled > correctly after a due fixup by neither of our handlers. I admit I should go look at the code, but for example RAM p2m types don't require this fix, so I assume there's some different path taken in that case that avoids all this? Ie: when global logdirty is enabled you will start to get nested page faults for every access, yet only direct MMIO types require this fix? Thanks, Roger
Re: [PATCH v2] x86: use POPCNT for hweight() when available
On Fri, May 22, 2020 at 11:58:40AM +0200, Jan Beulich wrote: > On 20.05.2020 19:18, Roger Pau Monné wrote: > > I also assume that using no_caller_saved_registers when available or > > else keeping the current behavior is not an acceptable solution? FWIW, > > from a FreeBSD PoV I would be OK with that, as I don't think there are > > any supported targets with clang < 5. > > By "current behavior" do you mean what the patch currently > does (matching my earlier response that I may try going this > route) or the unpatched upstream tree? Sorry this wasn't clear. By current I meant what's in the upstream tree. IMO having both no_caller_saved_registers, the -ffixed-reg stuff and the current upstream no popcnt implementation is kind of too much divergence? My preference would be to add popcnt support only when no_caller_saved_registers is supported by the compiler, as I think that's the cleanest solution. This is purely a performance optimization, so it doesn't seem that bad to me to request a newish compiler in order to enable it. Roger.
Re: [PATCH] x86/svm: retry after unhandled NPT fault if gfn was marked for recalculation
On 22/05/2020 11:05, Igor Druzhinin wrote: > On 22/05/2020 10:45, Andrew Cooper wrote: >> On 21/05/2020 22:43, Igor Druzhinin wrote: >>> If a recalculation NPT fault hasn't been handled explicitly in >>> hvm_hap_nested_page_fault() then it's potentially safe to retry - >>> US bit has been re-instated in PTE and any real fault would be correctly >>> re-raised next time. >>> >>> This covers a specific case of migration with vGPU assigned on AMD: >>> global log-dirty is enabled and causes immediate recalculation NPT >>> fault in MMIO area upon access. This type of fault isn't described >>> explicitly in hvm_hap_nested_page_fault (this isn't called on >>> EPT misconfig exit on Intel) which results in domain crash. >>> >>> Signed-off-by: Igor Druzhinin >>> --- >>> xen/arch/x86/hvm/svm/svm.c | 4 >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c >>> index 46a1aac..f0d0bd3 100644 >>> --- a/xen/arch/x86/hvm/svm/svm.c >>> +++ b/xen/arch/x86/hvm/svm/svm.c >>> @@ -1726,6 +1726,10 @@ static void svm_do_nested_pgfault(struct vcpu *v, >>> /* inject #VMEXIT(NPF) into guest. */ >>> nestedsvm_vmexit_defer(v, VMEXIT_NPF, pfec, gpa); >>> return; >>> +case 0: >>> +/* If a recalculation page fault hasn't been handled - just retry. >>> */ >>> +if ( pfec & PFEC_user_mode ) >>> +return; >> This smells like it is a recipe for livelocks. >> >> Everything should have been handled properly by the call to >> p2m_pt_handle_deferred_changes() which precedes svm_do_nested_pgfault(). >> >> It is legitimate for the MMIO mapping to end up being transiently >> recalculated, but the fact that p2m_pt_handle_deferred_changes() doesn't >> fix it up suggests that the bug is there. >> >> Do you have the complete NPT walk to the bad mapping? Do we have >> _PAGE_USER in the leaf mapping, or is this perhaps a spurious fault? > It does fix it up. The problem is that currently in SVM we enter > svm_do_nested_pgfault immediately after p2m_pt_handle_deferred_changes > is finished finished. Oh - so we do. I'd read the entry condition for svm_do_nested_pgfault() incorrectly. Jan - why did you chose to do it this way? If p2m_pt_handle_deferred_changes() has made a modification, there is surely nothing relevant to do in svm_do_nested_pgfault(). ~Andrew
Re: [PATCH] x86/svm: retry after unhandled NPT fault if gfn was marked for recalculation
On 22/05/2020 11:08, Roger Pau Monné wrote: > On Thu, May 21, 2020 at 10:43:58PM +0100, Igor Druzhinin wrote: >> If a recalculation NPT fault hasn't been handled explicitly in >> hvm_hap_nested_page_fault() then it's potentially safe to retry - >> US bit has been re-instated in PTE and any real fault would be correctly >> re-raised next time. >> >> This covers a specific case of migration with vGPU assigned on AMD: >> global log-dirty is enabled and causes immediate recalculation NPT >> fault in MMIO area upon access. This type of fault isn't described >> explicitly in hvm_hap_nested_page_fault (this isn't called on >> EPT misconfig exit on Intel) which results in domain crash. > > Couldn't direct MMIO regions be handled like other types of memory for > the purposes of logdiry mode? > > I assume there's already a path here used for other memory types when > logdirty is turned on, and hence would seem better to just make direct > MMIO regions also use that path? The proble of handling only MMIO case is that the issue still stays. It will be hit with some other memory type since it's not MMIO specific. The issue is that if global recalculation is called, the next hit to this type will cause a transient fault which will not be handled correctly after a due fixup by neither of our handlers. >> Signed-off-by: Igor Druzhinin >> --- >> xen/arch/x86/hvm/svm/svm.c | 4 >> 1 file changed, 4 insertions(+) >> >> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c >> index 46a1aac..f0d0bd3 100644 >> --- a/xen/arch/x86/hvm/svm/svm.c >> +++ b/xen/arch/x86/hvm/svm/svm.c >> @@ -1726,6 +1726,10 @@ static void svm_do_nested_pgfault(struct vcpu *v, >> /* inject #VMEXIT(NPF) into guest. */ >> nestedsvm_vmexit_defer(v, VMEXIT_NPF, pfec, gpa); >> return; >> +case 0: >> +/* If a recalculation page fault hasn't been handled - just retry. >> */ >> +if ( pfec & PFEC_user_mode ) >> +return; > > I'm slightly worried that this diverges from the EPT implementation > now, in the sense that returning 0 from hvm_hap_nested_page_fault will > no longer trigger a guest crash. My second alternative from my follow up email addresses this. I also didn't like this aspect. Igor
Re: [PATCH v4] public/io/netif.h: add a new extra type for XDP
On 5/22/20 12:33 PM, Denis Kirjanov wrote:
> On 5/22/20, Oleksandr Andrushchenko wrote:
>> On 5/22/20 12:17 PM, Denis Kirjanov wrote:
>>> On 5/22/20, Oleksandr Andrushchenko
>>> wrote:
On 5/18/20 6:04 PM, Denis Kirjanov wrote:
> The patch adds a new extra type to be able to diffirentiate
> between RX responses on xen-netfront side with the adjusted offset
> required for XDP processing.
>
> The offset value from a guest is passed via xenstore.
>
> Signed-off-by: Denis Kirjanov
> ---
> v4:
> - updated the commit and documenation
>
> v3:
> - updated the commit message
>
> v2:
> - added documentation
> - fixed padding for netif_extra_info
> ---
> ---
> xen/include/public/io/netif.h | 18 +-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/xen/include/public/io/netif.h
> b/xen/include/public/io/netif.h
> index 9fcf91a..a92bf04 100644
> --- a/xen/include/public/io/netif.h
> +++ b/xen/include/public/io/netif.h
> @@ -161,6 +161,17 @@
> */
>
> /*
> + * "xdp-headroom" is used to request that extra space is added
> + * for XDP processing. The value is measured in bytes and passed by
not sure that we should use word "bytes" here as the rest of the
protocol (mostly)
talks about octets. It is somewhat mixed here, no strong opinion
>>> sure, but since the public header mixes it I've decided to use that word.
>>>
>>>
> + * the frontend to be consistent between both ends.
> + * If the value is greater than zero that means that
> + * an RX response is going to be passed to an XDP program for
> processing.
> + *
> + * "feature-xdp-headroom" is set to "1" by the netback side like other
> features
> + * so a guest can check if an XDP program can be processed.
> + */
> +
> +/*
> * Control ring
> *
> *
> @@ -985,7 +996,8 @@ typedef struct netif_tx_request netif_tx_request_t;
> #define XEN_NETIF_EXTRA_TYPE_MCAST_ADD (2) /* u.mcast */
> #define XEN_NETIF_EXTRA_TYPE_MCAST_DEL (3) /* u.mcast */
> #define XEN_NETIF_EXTRA_TYPE_HASH (4) /* u.hash */
> -#define XEN_NETIF_EXTRA_TYPE_MAX (5)
> +#define XEN_NETIF_EXTRA_TYPE_XDP (5) /* u.xdp */
> +#define XEN_NETIF_EXTRA_TYPE_MAX (6)
>
> /* netif_extra_info_t flags. */
> #define _XEN_NETIF_EXTRA_FLAG_MORE (0)
> @@ -1018,6 +1030,10 @@ struct netif_extra_info {
> uint8_t algorithm;
> uint8_t value[4];
> } hash;
> +struct {
> +uint16_t headroom;
why do you need "pad" field here?
>>> To state that we have a fixed size available.
>> Well, I would expect "reserved" or something in that case and "pad" in case
>>
>> there are other fields following (see gso above).
> it can be consistent with other names like pad at then end of the structure.
>
> If it really matters I can change it, no problem.
My point is that IMO it is not required at all, but this is up to
maintainers to decide
>
>> But here I think "pad" is not required, just like mcast doesn't add any
> because it's already 6-bytes long
you are right
>
> +uint16_t pad[2]
> +} xdp;
> uint16_t pad[3];
> } u;
> };
Re: [PATCH] x86/svm: retry after unhandled NPT fault if gfn was marked for recalculation
On Thu, May 21, 2020 at 10:43:58PM +0100, Igor Druzhinin wrote: > If a recalculation NPT fault hasn't been handled explicitly in > hvm_hap_nested_page_fault() then it's potentially safe to retry - > US bit has been re-instated in PTE and any real fault would be correctly > re-raised next time. > > This covers a specific case of migration with vGPU assigned on AMD: > global log-dirty is enabled and causes immediate recalculation NPT > fault in MMIO area upon access. This type of fault isn't described > explicitly in hvm_hap_nested_page_fault (this isn't called on > EPT misconfig exit on Intel) which results in domain crash. Couldn't direct MMIO regions be handled like other types of memory for the purposes of logdiry mode? I assume there's already a path here used for other memory types when logdirty is turned on, and hence would seem better to just make direct MMIO regions also use that path? > Signed-off-by: Igor Druzhinin > --- > xen/arch/x86/hvm/svm/svm.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c > index 46a1aac..f0d0bd3 100644 > --- a/xen/arch/x86/hvm/svm/svm.c > +++ b/xen/arch/x86/hvm/svm/svm.c > @@ -1726,6 +1726,10 @@ static void svm_do_nested_pgfault(struct vcpu *v, > /* inject #VMEXIT(NPF) into guest. */ > nestedsvm_vmexit_defer(v, VMEXIT_NPF, pfec, gpa); > return; > +case 0: > +/* If a recalculation page fault hasn't been handled - just retry. */ > +if ( pfec & PFEC_user_mode ) > +return; I'm slightly worried that this diverges from the EPT implementation now, in the sense that returning 0 from hvm_hap_nested_page_fault will no longer trigger a guest crash. Thanks, Roger.
Re: [PATCH v3] x86/PV: remove unnecessary toggle_guest_pt() overhead
On 21.05.2020 18:46, Andrew Cooper wrote: > On 05/05/2020 07:16, Jan Beulich wrote: >> While the mere updating of ->pv_cr3 and ->root_pgt_changed aren't overly >> expensive (but still needed only for the toggle_guest_mode() path), the >> effect of the latter on the exit-to-guest path is not insignificant. >> Move the logic into toggle_guest_mode(), on the basis that >> toggle_guest_pt() will always be invoked in pairs, yet we can't safely >> undo the setting of root_pgt_changed during the second of these >> invocations. >> >> While at it, add a comment ahead of toggle_guest_pt() to clarify its >> intended usage. >> >> Signed-off-by: Jan Beulich > > I'm still of the opinion that the commit message wants rewriting to get > the important points across clearly. > > And those are that toggle_guest_pt() is called in pairs specifically to > read kernel data structures when emulating a userspace action, and that > this doesn't modify cr3 from the guests point of view, and therefore > doesn't need the resync on exit-to-guest path. Is this "toggle_guest_pt() is called in pairs, to read guest kernel data structures when emulating a guest userspace action. Hence this doesn't modify cr3 from the guest's point of view, and therefore doesn't need any resync on the exit-to-guest path. Therefore move the updating of ->pv_cr3 and ->root_pgt_changed into toggle_guest_mode(), since undoing the changes during the second of these invocations wouldn't be a safe thing to do." any better? Jan
Re: [PATCH] x86/svm: retry after unhandled NPT fault if gfn was marked for recalculation
On 22/05/2020 10:45, Andrew Cooper wrote: > On 21/05/2020 22:43, Igor Druzhinin wrote: >> If a recalculation NPT fault hasn't been handled explicitly in >> hvm_hap_nested_page_fault() then it's potentially safe to retry - >> US bit has been re-instated in PTE and any real fault would be correctly >> re-raised next time. >> >> This covers a specific case of migration with vGPU assigned on AMD: >> global log-dirty is enabled and causes immediate recalculation NPT >> fault in MMIO area upon access. This type of fault isn't described >> explicitly in hvm_hap_nested_page_fault (this isn't called on >> EPT misconfig exit on Intel) which results in domain crash. >> >> Signed-off-by: Igor Druzhinin >> --- >> xen/arch/x86/hvm/svm/svm.c | 4 >> 1 file changed, 4 insertions(+) >> >> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c >> index 46a1aac..f0d0bd3 100644 >> --- a/xen/arch/x86/hvm/svm/svm.c >> +++ b/xen/arch/x86/hvm/svm/svm.c >> @@ -1726,6 +1726,10 @@ static void svm_do_nested_pgfault(struct vcpu *v, >> /* inject #VMEXIT(NPF) into guest. */ >> nestedsvm_vmexit_defer(v, VMEXIT_NPF, pfec, gpa); >> return; >> +case 0: >> +/* If a recalculation page fault hasn't been handled - just retry. >> */ >> +if ( pfec & PFEC_user_mode ) >> +return; > > This smells like it is a recipe for livelocks. > > Everything should have been handled properly by the call to > p2m_pt_handle_deferred_changes() which precedes svm_do_nested_pgfault(). > > It is legitimate for the MMIO mapping to end up being transiently > recalculated, but the fact that p2m_pt_handle_deferred_changes() doesn't > fix it up suggests that the bug is there. > > Do you have the complete NPT walk to the bad mapping? Do we have > _PAGE_USER in the leaf mapping, or is this perhaps a spurious fault? It does fix it up. The problem is that currently in SVM we enter svm_do_nested_pgfault immediately after p2m_pt_handle_deferred_changes is finished finished. Yes, we don't have _PAGE_USER initially and, yes, it's fixed up correctly in p2m_pt_handle_deferred_changes but svm_do_nested_pgfault doesn't know about it. Please read my second email about alternatives that suggest to resolve the issue you're worrying about. Igor
Re: [PATCH] golang: Update generated files after libxl_types.idl change
On Fri, May 22, 2020 at 10:49:56AM +0100, George Dunlap wrote:
> c/s 7efd9f3d45 ("libxl: Handle Linux stubdomain specific QEMU
> options.") modified libl_types.idl. Run gengotypes.py again to update
libxl_types.idl.
> the geneated golang bindings.
>
Can we perhaps add a dependency on golang's side such that it can be
auto-generated in the future?
In any case
Acked-by: Wei Liu
(I haven't looked at the generated code)
Re: [PATCH 2/3] configure: also add EXTRA_PREFIX to {CPP/LD}FLAGS
On Fri, May 22, 2020 at 09:37:51AM +, Bertrand Marquis wrote: > Hi, > > > On 22 May 2020, at 10:05, Wei Liu wrote: > > > > On Fri, May 22, 2020 at 08:41:17AM +, Bertrand Marquis wrote: > >> Hi, > >> > >> As a consequence of this fix, the following has been committed (I guess as > >> a consequence of regenerating the configure scripts): > >> diff --git a/tools/configure b/tools/configure > >> index 375430df3f..36596389b8 100755 > >> --- a/tools/configure > >> +++ b/tools/configure > >> @@ -4678,6 +4678,10 @@ for ldflag in $APPEND_LIB > >> do > >> APPEND_LDFLAGS="$APPEND_LDFLAGS -L$ldflag" > >> done > >> +if ! -z $EXTRA_PREFIX ; then > >> +CPPFLAGS="$CPPFLAGS -I$EXTRA_PREFIX/include" > >> +LDFLAGS="$LDFLAGS -L$EXTRA_PREFIX/lib" > >> +fi > >> CPPFLAGS="$PREPEND_CPPFLAGS $CPPFLAGS $APPEND_CPPFLAGS" > >> LDFLAGS="$PREPEND_LDFLAGS $LDFLAGS $APPEND_LDFLAGS” > >> > >> This should be: > >> if [ ! -z $EXTRA_PREFIX ]; then > >> > >> As on other configure scripts. > >> > >> During configure I have not the following error: > >> ./configure: line 4681: -z: command not found > >> > >> Which is ignored but is adding -L/lib and -I/include to the CPPFLAGS and > >> LDFLAGS > >> > >> What should be the procedure to actually fix that (as the problem is > >> coming from the configure script regeneration I guess) ? > > > > Does the following patch work for you? > > > > diff --git a/m4/set_cflags_ldflags.m4 b/m4/set_cflags_ldflags.m4 > > index 08f5c983cc63..cd34c139bc94 100644 > > --- a/m4/set_cflags_ldflags.m4 > > +++ b/m4/set_cflags_ldflags.m4 > > @@ -15,7 +15,7 @@ for ldflag in $APPEND_LIB > > do > > APPEND_LDFLAGS="$APPEND_LDFLAGS -L$ldflag" > > done > > -if [ ! -z $EXTRA_PREFIX ]; then > > +if test ! -z $EXTRA_PREFIX ; then > > CPPFLAGS="$CPPFLAGS -I$EXTRA_PREFIX/include" > > LDFLAGS="$LDFLAGS -L$EXTRA_PREFIX/lib" > > fi > > > > > > You will need to run autogen.sh to regenerate tools/configure. > > > > Yes that works on my side and generate tools/configure using “test” > > But why are the [] being removed when generating tools/configure ? No idea why autoconf removed [] really. I think switching to test is better anyway since that's what is used throughout tools/configure. Wei.
Re: [PATCH v2] x86: use POPCNT for hweight() when available
On 20.05.2020 19:18, Roger Pau Monné wrote: > I also assume that using no_caller_saved_registers when available or > else keeping the current behavior is not an acceptable solution? FWIW, > from a FreeBSD PoV I would be OK with that, as I don't think there are > any supported targets with clang < 5. By "current behavior" do you mean what the patch currently does (matching my earlier response that I may try going this route) or the unpatched upstream tree? Jan
RE: [PATCH v7 00/19] Add support for qemu-xen runnning in a Linux-based stubdomain
> -Original Message- > From: Xen-devel On Behalf Of George > Dunlap > Sent: 22 May 2020 10:11 > To: Jason Andryuk > Cc: Stefano Stabellini ; Julien Grall > ; Samuel Thibault > ; Wei Liu ; Andrew Cooper > ; Jan > Beulich ; Ian Jackson ; Anthony > Perard > ; xen-devel ; > Daniel De Graaf > > Subject: Re: [PATCH v7 00/19] Add support for qemu-xen runnning in a > Linux-based stubdomain > > > > On May 19, 2020, at 2:54 AM, Jason Andryuk wrote: > > > > General idea is to allow freely set device_model_version and > > device_model_stubdomain_override and choose the right options based on this > > choice. Also, allow to specific path to stubdomain kernel/ramdisk, for > > greater > > flexibility. > > Excited to see this patch series get in. But I didn’t really notice any > documents explaining how to > actually use it — is there a blog post anywhere describing how to get the > kernel / initrd image and so > on? > > Also, would it be possible to add a follow-up series which modifies > SUPPORT.md and CHANGELOG.md? Yes please. In future I think we should encourage the patch to CHANGELOG.md to be the last patch of a series such as this. Paul > > Thanks, > -George
Re: [PATCH] x86: refine guest_mode()
On 20.05.2020 17:13, Roger Pau Monné wrote: > On Wed, May 20, 2020 at 10:56:26AM +0200, Jan Beulich wrote: >> On 18.05.2020 16:51, Roger Pau Monné wrote: >>> On Tue, Apr 28, 2020 at 08:30:12AM +0200, Jan Beulich wrote: On 27.04.2020 22:11, Andrew Cooper wrote: > On 27/04/2020 16:15, Jan Beulich wrote: >> On 27.04.2020 16:35, Andrew Cooper wrote: >>> On 27/04/2020 09:03, Jan Beulich wrote: --- a/xen/include/asm-x86/regs.h +++ b/xen/include/asm-x86/regs.h @@ -10,9 +10,10 @@ /* Frame pointer must point into current CPU stack. */ \ ASSERT(diff < STACK_SIZE); \ /* If not a guest frame, it must be a hypervisor frame. */ \ -ASSERT((diff == 0) || (r->cs == __HYPERVISOR_CS)); \ +if ( diff < PRIMARY_STACK_SIZE ) \ +ASSERT(!diff || ((r)->cs == __HYPERVISOR_CS)); \ /* Return TRUE if it's a guest frame. */ \ -(diff == 0); \ +!diff || ((r)->cs != __HYPERVISOR_CS); \ >>> The (diff == 0) already worried me before because it doesn't fail safe, >>> but this makes things more problematic. Consider the case back when we >>> had __HYPERVISOR_CS32. >> Yes - if __HYPERVISOR_CS32 would ever have been to be used for >> anything, it would have needed checking for here. >> >>> Guest mode is strictly "(r)->cs & 3". >> As long as CS (a) gets properly saved (it's a "manual" step for >> SYSCALL/SYSRET as well as #VMEXIT) and (b) didn't get clobbered. I >> didn't write this code, I don't think, so I can only guess that >> there were intentions behind this along these lines. > > Hmm - the VMExit case might be problematic here, due to the variability > in the poison used. "Variability" is an understatement - there's no poisoning at all in release builds afaics (and to be honest it seems a somewhat pointless to write the same values over and over again in debug mode). With this, ... >>> Everything else is expectations about how things ought to be laid out, >>> but for safety in release builds, the final judgement should not depend >>> on the expectations evaluating true. >> Well, I can switch to a purely CS.RPL based approach, as long as >> we're happy to live with the possible downside mentioned above. >> Of course this would then end up being a more intrusive change >> than originally intended ... > > I'd certainly prefer to go for something which is more robust, even if > it is a larger change. ... what's your suggestion? Basing on _just_ CS.RPL obviously won't work. Not even if we put in place the guest's CS (albeit that somewhat depends on the meaning we assign to the macro's returned value). >>> >>> Just to check I'm following this correctly, using CS.RPL won't work >>> for HVM guests, as HVM can legitimately use a RPL of 0 (which is not >>> the case for PV guests). Doesn't the same apply to the usage of >>> __HYPERVISOR_CS? (A HVM guest could also use the same code segment >>> value as Xen?) >> >> Of course (and in particular Xen as a guest would). My "Basing on >> _just_ CS.RPL" wasn't meant to exclude the rest of the selector, >> but to contrast this to the case where "diff" also is involved in >> the calculation (which looks to be what Andrew would prefer to see >> go away). >> Using current inside the macro to determine whether the guest is HVM would also seem fragile to me - there are quite a few uses of guest_mode(). Which would leave passing in a const struct vcpu * (or domain *), requiring to touch all call sites, including Arm's. >>> >>> Fragile or slow? Are there corner cases where guest_mode is used where >>> current is not reliable? >> >> This question is why I said "there are quite a few uses of >> guest_mode()" - auditing them all is just one side of the medal. >> The other is to prevent a new use appearing in the future that >> can be reached by a call path in the time window where a lazy >> context switch is pending (i.e. when current has already been >> updated, but register state hasn't been yet). >> Compared to this it would seem to me that the change as presented is a clear improvement without becoming overly large of a change. >>> >>> Using the cs register is already part of the guest_mode code, even if >>> just in debug mode, hence I don't see it as a regression from existing >>> code. It however feels weird to me that the reporter of the issue >>> doesn't agree with the fix, and
[PATCH] golang: Update generated files after libxl_types.idl change
c/s 7efd9f3d45 ("libxl: Handle Linux stubdomain specific QEMU
options.") modified libl_types.idl. Run gengotypes.py again to update
the geneated golang bindings.
Signed-off-by: George Dunlap
---
CC: Nick Rosbrook
CC: Ian Jackson
CC: Wei Liu
---
tools/golang/xenlight/helpers.gen.go | 10 ++
tools/golang/xenlight/types.gen.go | 3 +++
2 files changed, 13 insertions(+)
diff --git a/tools/golang/xenlight/helpers.gen.go
b/tools/golang/xenlight/helpers.gen.go
index 109e9515a2..b5bd0de830 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -1163,6 +1163,9 @@ func (x *DomainBuildInfo) fromC(xc
*C.libxl_domain_build_info) error {
if err := x.DeviceModelStubdomain.fromC(_model_stubdomain);
err != nil {
return fmt.Errorf("converting field DeviceModelStubdomain: %v",
err)
}
+ x.StubdomainMemkb = uint64(xc.stubdomain_memkb)
+ x.StubdomainKernel = C.GoString(xc.stubdomain_kernel)
+ 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)
@@ -1489,6 +1492,13 @@ func (x *DomainBuildInfo) toC(xc
*C.libxl_domain_build_info) (err error) {
if err := x.DeviceModelStubdomain.toC(_model_stubdomain); err
!= nil {
return fmt.Errorf("converting field DeviceModelStubdomain: %v",
err)
}
+ xc.stubdomain_memkb = C.uint64_t(x.StubdomainMemkb)
+ if x.StubdomainKernel != "" {
+ xc.stubdomain_kernel = C.CString(x.StubdomainKernel)
+ }
+ if x.StubdomainRamdisk != "" {
+ xc.stubdomain_ramdisk = C.CString(x.StubdomainRamdisk)
+ }
if x.DeviceModel != "" {
xc.device_model = C.CString(x.DeviceModel)
}
diff --git a/tools/golang/xenlight/types.gen.go
b/tools/golang/xenlight/types.gen.go
index df68fd0e88..15516ae552 100644
--- a/tools/golang/xenlight/types.gen.go
+++ b/tools/golang/xenlight/types.gen.go
@@ -509,6 +509,9 @@ type DomainBuildInfo struct {
MaxMaptrackFrames uint32
DeviceModelVersionDeviceModelVersion
DeviceModelStubdomain Defbool
+ StubdomainMemkb uint64
+ StubdomainKernel string
+ StubdomainRamdisk string
DeviceModel string
DeviceModelSsidrefuint32
DeviceModelSsidLabel string
--
2.25.1
Re: [PATCH] x86/svm: retry after unhandled NPT fault if gfn was marked for recalculation
On 21/05/2020 22:43, Igor Druzhinin wrote: > If a recalculation NPT fault hasn't been handled explicitly in > hvm_hap_nested_page_fault() then it's potentially safe to retry - > US bit has been re-instated in PTE and any real fault would be correctly > re-raised next time. > > This covers a specific case of migration with vGPU assigned on AMD: > global log-dirty is enabled and causes immediate recalculation NPT > fault in MMIO area upon access. This type of fault isn't described > explicitly in hvm_hap_nested_page_fault (this isn't called on > EPT misconfig exit on Intel) which results in domain crash. > > Signed-off-by: Igor Druzhinin > --- > xen/arch/x86/hvm/svm/svm.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c > index 46a1aac..f0d0bd3 100644 > --- a/xen/arch/x86/hvm/svm/svm.c > +++ b/xen/arch/x86/hvm/svm/svm.c > @@ -1726,6 +1726,10 @@ static void svm_do_nested_pgfault(struct vcpu *v, > /* inject #VMEXIT(NPF) into guest. */ > nestedsvm_vmexit_defer(v, VMEXIT_NPF, pfec, gpa); > return; > +case 0: > +/* If a recalculation page fault hasn't been handled - just retry. */ > +if ( pfec & PFEC_user_mode ) > +return; This smells like it is a recipe for livelocks. Everything should have been handled properly by the call to p2m_pt_handle_deferred_changes() which precedes svm_do_nested_pgfault(). It is legitimate for the MMIO mapping to end up being transiently recalculated, but the fact that p2m_pt_handle_deferred_changes() doesn't fix it up suggests that the bug is there. Do you have the complete NPT walk to the bad mapping? Do we have _PAGE_USER in the leaf mapping, or is this perhaps a spurious fault? ~Andrew
RE: [PATCH] golang/xenlight: add an empty line after DO NOT EDIT comment
> -Original Message- > From: George Dunlap > Sent: 22 May 2020 10:14 > To: Nick Rosbrook > Cc: xen-devel ; Nick Rosbrook > ; Ian Jackson > ; Wei Liu ; Paul Durrant > Subject: Re: [PATCH] golang/xenlight: add an empty line after DO NOT EDIT > comment > > CC’ing the release manager, since we’re past the last posting date > > > On May 21, 2020, at 3:55 PM, Nick Rosbrook wrote: > > > > When generating documentation, pkg.go.dev and godoc.org assume a comment > > that immediately precedes the package declaration is a "package > > comment", and should be shown in the documentation. Add an empty line > > after the DO NOT EDIT comment in generated files to prevent these > > comments from appearing as "package comments." > > > > Signed-off-by: Nick Rosbrook > > Reviewed-by: George Dunlap > > Paul, I would classify this as a bug fix: It won’t have any functional effect > on the code itself, but > it fixes how it’s displayed; e.g.: > > https://pkg.go.dev/xenbits.xenproject.org/git-http/xen.git/tools/golang/xenlight?tab=doc > Since it is apparently a pure whitespace change I have no problem with this going in. We're not at freeze yet so technically you don't need my release-ack as yet :-) Paul
Re: [PATCH 2/3] configure: also add EXTRA_PREFIX to {CPP/LD}FLAGS
Hi, > On 22 May 2020, at 10:05, Wei Liu wrote: > > On Fri, May 22, 2020 at 08:41:17AM +, Bertrand Marquis wrote: >> Hi, >> >> As a consequence of this fix, the following has been committed (I guess as a >> consequence of regenerating the configure scripts): >> diff --git a/tools/configure b/tools/configure >> index 375430df3f..36596389b8 100755 >> --- a/tools/configure >> +++ b/tools/configure >> @@ -4678,6 +4678,10 @@ for ldflag in $APPEND_LIB >> do >> APPEND_LDFLAGS="$APPEND_LDFLAGS -L$ldflag" >> done >> +if ! -z $EXTRA_PREFIX ; then >> +CPPFLAGS="$CPPFLAGS -I$EXTRA_PREFIX/include" >> +LDFLAGS="$LDFLAGS -L$EXTRA_PREFIX/lib" >> +fi >> CPPFLAGS="$PREPEND_CPPFLAGS $CPPFLAGS $APPEND_CPPFLAGS" >> LDFLAGS="$PREPEND_LDFLAGS $LDFLAGS $APPEND_LDFLAGS” >> >> This should be: >> if [ ! -z $EXTRA_PREFIX ]; then >> >> As on other configure scripts. >> >> During configure I have not the following error: >> ./configure: line 4681: -z: command not found >> >> Which is ignored but is adding -L/lib and -I/include to the CPPFLAGS and >> LDFLAGS >> >> What should be the procedure to actually fix that (as the problem is coming >> from the configure script regeneration I guess) ? > > Does the following patch work for you? > > diff --git a/m4/set_cflags_ldflags.m4 b/m4/set_cflags_ldflags.m4 > index 08f5c983cc63..cd34c139bc94 100644 > --- a/m4/set_cflags_ldflags.m4 > +++ b/m4/set_cflags_ldflags.m4 > @@ -15,7 +15,7 @@ for ldflag in $APPEND_LIB > do > APPEND_LDFLAGS="$APPEND_LDFLAGS -L$ldflag" > done > -if [ ! -z $EXTRA_PREFIX ]; then > +if test ! -z $EXTRA_PREFIX ; then > CPPFLAGS="$CPPFLAGS -I$EXTRA_PREFIX/include" > LDFLAGS="$LDFLAGS -L$EXTRA_PREFIX/lib" > fi > > > You will need to run autogen.sh to regenerate tools/configure. > Yes that works on my side and generate tools/configure using “test” But why are the [] being removed when generating tools/configure ? Bertrand > Wei. > >> >> Bertrand >> >>> On 5 May 2020, at 10:24, Roger Pau Monne wrote: >>> >>> The path provided by EXTRA_PREFIX should be added to the search path >>> of the configure script, like it's done in Config.mk. Not doing so >>> makes the search path for configure differ from the search path used >>> by the build. >>> >>> Signed-off-by: Roger Pau Monné >>> --- >>> Please re-run autoconf.sh after applying. >>> --- >>> m4/set_cflags_ldflags.m4 | 4 >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/m4/set_cflags_ldflags.m4 b/m4/set_cflags_ldflags.m4 >>> index cbad3c10b0..08f5c983cc 100644 >>> --- a/m4/set_cflags_ldflags.m4 >>> +++ b/m4/set_cflags_ldflags.m4 >>> @@ -15,6 +15,10 @@ for ldflag in $APPEND_LIB >>> do >>>APPEND_LDFLAGS="$APPEND_LDFLAGS -L$ldflag" >>> done >>> +if [ ! -z $EXTRA_PREFIX ]; then >>> +CPPFLAGS="$CPPFLAGS -I$EXTRA_PREFIX/include" >>> +LDFLAGS="$LDFLAGS -L$EXTRA_PREFIX/lib" >>> +fi >>> CPPFLAGS="$PREPEND_CPPFLAGS $CPPFLAGS $APPEND_CPPFLAGS" >>> LDFLAGS="$PREPEND_LDFLAGS $LDFLAGS $APPEND_LDFLAGS"]) >>> >>> -- >>> 2.26.2
Re: [PATCH v4] public/io/netif.h: add a new extra type for XDP
On 5/22/20, Oleksandr Andrushchenko wrote:
> On 5/22/20 12:17 PM, Denis Kirjanov wrote:
>> On 5/22/20, Oleksandr Andrushchenko
>> wrote:
>>> On 5/18/20 6:04 PM, Denis Kirjanov wrote:
The patch adds a new extra type to be able to diffirentiate
between RX responses on xen-netfront side with the adjusted offset
required for XDP processing.
The offset value from a guest is passed via xenstore.
Signed-off-by: Denis Kirjanov
---
v4:
- updated the commit and documenation
v3:
- updated the commit message
v2:
- added documentation
- fixed padding for netif_extra_info
---
---
xen/include/public/io/netif.h | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/xen/include/public/io/netif.h
b/xen/include/public/io/netif.h
index 9fcf91a..a92bf04 100644
--- a/xen/include/public/io/netif.h
+++ b/xen/include/public/io/netif.h
@@ -161,6 +161,17 @@
*/
/*
+ * "xdp-headroom" is used to request that extra space is added
+ * for XDP processing. The value is measured in bytes and passed by
>>> not sure that we should use word "bytes" here as the rest of the
>>> protocol (mostly)
>>>
>>> talks about octets. It is somewhat mixed here, no strong opinion
>> sure, but since the public header mixes it I've decided to use that word.
>>
>>
+ * the frontend to be consistent between both ends.
+ * If the value is greater than zero that means that
+ * an RX response is going to be passed to an XDP program for
processing.
+ *
+ * "feature-xdp-headroom" is set to "1" by the netback side like other
features
+ * so a guest can check if an XDP program can be processed.
+ */
+
+/*
* Control ring
*
*
@@ -985,7 +996,8 @@ typedef struct netif_tx_request netif_tx_request_t;
#define XEN_NETIF_EXTRA_TYPE_MCAST_ADD (2) /* u.mcast */
#define XEN_NETIF_EXTRA_TYPE_MCAST_DEL (3) /* u.mcast */
#define XEN_NETIF_EXTRA_TYPE_HASH (4) /* u.hash */
-#define XEN_NETIF_EXTRA_TYPE_MAX (5)
+#define XEN_NETIF_EXTRA_TYPE_XDP (5) /* u.xdp */
+#define XEN_NETIF_EXTRA_TYPE_MAX (6)
/* netif_extra_info_t flags. */
#define _XEN_NETIF_EXTRA_FLAG_MORE (0)
@@ -1018,6 +1030,10 @@ struct netif_extra_info {
uint8_t algorithm;
uint8_t value[4];
} hash;
+struct {
+uint16_t headroom;
>>> why do you need "pad" field here?
>> To state that we have a fixed size available.
>
> Well, I would expect "reserved" or something in that case and "pad" in case
>
> there are other fields following (see gso above).
it can be consistent with other names like pad at then end of the structure.
If it really matters I can change it, no problem.
>
> But here I think "pad" is not required, just like mcast doesn't add any
because it's already 6-bytes long
>
>>
+uint16_t pad[2]
+} xdp;
uint16_t pad[3];
} u;
};
Re: [PATCH v4] public/io/netif.h: add a new extra type for XDP
On 5/22/20 12:17 PM, Denis Kirjanov wrote:
> On 5/22/20, Oleksandr Andrushchenko wrote:
>> On 5/18/20 6:04 PM, Denis Kirjanov wrote:
>>> The patch adds a new extra type to be able to diffirentiate
>>> between RX responses on xen-netfront side with the adjusted offset
>>> required for XDP processing.
>>>
>>> The offset value from a guest is passed via xenstore.
>>>
>>> Signed-off-by: Denis Kirjanov
>>> ---
>>> v4:
>>> - updated the commit and documenation
>>>
>>> v3:
>>> - updated the commit message
>>>
>>> v2:
>>> - added documentation
>>> - fixed padding for netif_extra_info
>>> ---
>>> ---
>>>xen/include/public/io/netif.h | 18 +-
>>>1 file changed, 17 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/xen/include/public/io/netif.h
>>> b/xen/include/public/io/netif.h
>>> index 9fcf91a..a92bf04 100644
>>> --- a/xen/include/public/io/netif.h
>>> +++ b/xen/include/public/io/netif.h
>>> @@ -161,6 +161,17 @@
>>> */
>>>
>>>/*
>>> + * "xdp-headroom" is used to request that extra space is added
>>> + * for XDP processing. The value is measured in bytes and passed by
>> not sure that we should use word "bytes" here as the rest of the
>> protocol (mostly)
>>
>> talks about octets. It is somewhat mixed here, no strong opinion
> sure, but since the public header mixes it I've decided to use that word.
>
>
>>> + * the frontend to be consistent between both ends.
>>> + * If the value is greater than zero that means that
>>> + * an RX response is going to be passed to an XDP program for
>>> processing.
>>> + *
>>> + * "feature-xdp-headroom" is set to "1" by the netback side like other
>>> features
>>> + * so a guest can check if an XDP program can be processed.
>>> + */
>>> +
>>> +/*
>>> * Control ring
>>> *
>>> *
>>> @@ -985,7 +996,8 @@ typedef struct netif_tx_request netif_tx_request_t;
>>>#define XEN_NETIF_EXTRA_TYPE_MCAST_ADD (2) /* u.mcast */
>>>#define XEN_NETIF_EXTRA_TYPE_MCAST_DEL (3) /* u.mcast */
>>>#define XEN_NETIF_EXTRA_TYPE_HASH (4) /* u.hash */
>>> -#define XEN_NETIF_EXTRA_TYPE_MAX (5)
>>> +#define XEN_NETIF_EXTRA_TYPE_XDP (5) /* u.xdp */
>>> +#define XEN_NETIF_EXTRA_TYPE_MAX (6)
>>>
>>>/* netif_extra_info_t flags. */
>>>#define _XEN_NETIF_EXTRA_FLAG_MORE (0)
>>> @@ -1018,6 +1030,10 @@ struct netif_extra_info {
>>>uint8_t algorithm;
>>>uint8_t value[4];
>>>} hash;
>>> +struct {
>>> +uint16_t headroom;
>> why do you need "pad" field here?
> To state that we have a fixed size available.
Well, I would expect "reserved" or something in that case and "pad" in case
there are other fields following (see gso above).
But here I think "pad" is not required, just like mcast doesn't add any
>
>>> +uint16_t pad[2]
>>> +} xdp;
>>>uint16_t pad[3];
>>>} u;
>>>};
Re: [PATCH v4] public/io/netif.h: add a new extra type for XDP
On 5/18/20, Denis Kirjanov wrote:
> The patch adds a new extra type to be able to diffirentiate
> between RX responses on xen-netfront side with the adjusted offset
> required for XDP processing.
>
> The offset value from a guest is passed via xenstore.
I'm going to send a new version for Linux with the above changes applied.
>
> Signed-off-by: Denis Kirjanov
> ---
> v4:
> - updated the commit and documenation
>
> v3:
> - updated the commit message
>
> v2:
> - added documentation
> - fixed padding for netif_extra_info
> ---
> ---
> xen/include/public/io/netif.h | 18 +-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/xen/include/public/io/netif.h b/xen/include/public/io/netif.h
> index 9fcf91a..a92bf04 100644
> --- a/xen/include/public/io/netif.h
> +++ b/xen/include/public/io/netif.h
> @@ -161,6 +161,17 @@
> */
>
> /*
> + * "xdp-headroom" is used to request that extra space is added
> + * for XDP processing. The value is measured in bytes and passed by
> + * the frontend to be consistent between both ends.
> + * If the value is greater than zero that means that
> + * an RX response is going to be passed to an XDP program for processing.
> + *
> + * "feature-xdp-headroom" is set to "1" by the netback side like other
> features
> + * so a guest can check if an XDP program can be processed.
> + */
> +
> +/*
> * Control ring
> *
> *
> @@ -985,7 +996,8 @@ typedef struct netif_tx_request netif_tx_request_t;
> #define XEN_NETIF_EXTRA_TYPE_MCAST_ADD (2) /* u.mcast */
> #define XEN_NETIF_EXTRA_TYPE_MCAST_DEL (3) /* u.mcast */
> #define XEN_NETIF_EXTRA_TYPE_HASH (4) /* u.hash */
> -#define XEN_NETIF_EXTRA_TYPE_MAX (5)
> +#define XEN_NETIF_EXTRA_TYPE_XDP (5) /* u.xdp */
> +#define XEN_NETIF_EXTRA_TYPE_MAX (6)
>
> /* netif_extra_info_t flags. */
> #define _XEN_NETIF_EXTRA_FLAG_MORE (0)
> @@ -1018,6 +1030,10 @@ struct netif_extra_info {
> uint8_t algorithm;
> uint8_t value[4];
> } hash;
> +struct {
> +uint16_t headroom;
> +uint16_t pad[2]
> +} xdp;
> uint16_t pad[3];
> } u;
> };
> --
> 1.8.3.1
>
>
Re: [PATCH v4] public/io/netif.h: add a new extra type for XDP
On 5/22/20, Oleksandr Andrushchenko wrote:
> On 5/18/20 6:04 PM, Denis Kirjanov wrote:
>> The patch adds a new extra type to be able to diffirentiate
>> between RX responses on xen-netfront side with the adjusted offset
>> required for XDP processing.
>>
>> The offset value from a guest is passed via xenstore.
>>
>> Signed-off-by: Denis Kirjanov
>> ---
>> v4:
>> - updated the commit and documenation
>>
>> v3:
>> - updated the commit message
>>
>> v2:
>> - added documentation
>> - fixed padding for netif_extra_info
>> ---
>> ---
>> xen/include/public/io/netif.h | 18 +-
>> 1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/include/public/io/netif.h
>> b/xen/include/public/io/netif.h
>> index 9fcf91a..a92bf04 100644
>> --- a/xen/include/public/io/netif.h
>> +++ b/xen/include/public/io/netif.h
>> @@ -161,6 +161,17 @@
>>*/
>>
>> /*
>> + * "xdp-headroom" is used to request that extra space is added
>> + * for XDP processing. The value is measured in bytes and passed by
>
> not sure that we should use word "bytes" here as the rest of the
> protocol (mostly)
>
> talks about octets. It is somewhat mixed here, no strong opinion
sure, but since the public header mixes it I've decided to use that word.
>
>> + * the frontend to be consistent between both ends.
>> + * If the value is greater than zero that means that
>> + * an RX response is going to be passed to an XDP program for
>> processing.
>> + *
>> + * "feature-xdp-headroom" is set to "1" by the netback side like other
>> features
>> + * so a guest can check if an XDP program can be processed.
>> + */
>> +
>> +/*
>>* Control ring
>>*
>>*
>> @@ -985,7 +996,8 @@ typedef struct netif_tx_request netif_tx_request_t;
>> #define XEN_NETIF_EXTRA_TYPE_MCAST_ADD (2) /* u.mcast */
>> #define XEN_NETIF_EXTRA_TYPE_MCAST_DEL (3) /* u.mcast */
>> #define XEN_NETIF_EXTRA_TYPE_HASH (4) /* u.hash */
>> -#define XEN_NETIF_EXTRA_TYPE_MAX (5)
>> +#define XEN_NETIF_EXTRA_TYPE_XDP (5) /* u.xdp */
>> +#define XEN_NETIF_EXTRA_TYPE_MAX (6)
>>
>> /* netif_extra_info_t flags. */
>> #define _XEN_NETIF_EXTRA_FLAG_MORE (0)
>> @@ -1018,6 +1030,10 @@ struct netif_extra_info {
>> uint8_t algorithm;
>> uint8_t value[4];
>> } hash;
>> +struct {
>> +uint16_t headroom;
> why do you need "pad" field here?
To state that we have a fixed size available.
>> +uint16_t pad[2]
>> +} xdp;
>> uint16_t pad[3];
>> } u;
>> };
Re: [PATCH] x86/idle: prevent entering C3/C6 on some Intel CPUs due to errata
On 22/05/2020 09:09, Roger Pau Monne wrote: > Apply a workaround for errata BA80, AAK120, AAM108, AAO67, BD59, > AAY54: Rapid Core C3/C6 Transition May Cause Unpredictable System > Behavior. > > Limit maximum C state to C2 when SMT is enabled on the affected CPUs. C1 > Signed-off-by: Roger Pau Monné Acked-by: Andrew Cooper A fix for this is long overdue. ~Andrew
Re: [PATCH] golang/xenlight: add an empty line after DO NOT EDIT comment
CC’ing the release manager, since we’re past the last posting date > On May 21, 2020, at 3:55 PM, Nick Rosbrook wrote: > > When generating documentation, pkg.go.dev and godoc.org assume a comment > that immediately precedes the package declaration is a "package > comment", and should be shown in the documentation. Add an empty line > after the DO NOT EDIT comment in generated files to prevent these > comments from appearing as "package comments." > > Signed-off-by: Nick Rosbrook Reviewed-by: George Dunlap Paul, I would classify this as a bug fix: It won’t have any functional effect on the code itself, but it fixes how it’s displayed; e.g.: https://pkg.go.dev/xenbits.xenproject.org/git-http/xen.git/tools/golang/xenlight?tab=doc
Re: [PATCH v7 00/19] Add support for qemu-xen runnning in a Linux-based stubdomain
> On May 19, 2020, at 2:54 AM, Jason Andryuk wrote: > > General idea is to allow freely set device_model_version and > device_model_stubdomain_override and choose the right options based on this > choice. Also, allow to specific path to stubdomain kernel/ramdisk, for > greater > flexibility. Excited to see this patch series get in. But I didn’t really notice any documents explaining how to actually use it — is there a blog post anywhere describing how to get the kernel / initrd image and so on? Also, would it be possible to add a follow-up series which modifies SUPPORT.md and CHANGELOG.md? Thanks, -George
Re: [PATCH v4] public/io/netif.h: add a new extra type for XDP
On 5/18/20 6:04 PM, Denis Kirjanov wrote:
> The patch adds a new extra type to be able to diffirentiate
> between RX responses on xen-netfront side with the adjusted offset
> required for XDP processing.
>
> The offset value from a guest is passed via xenstore.
>
> Signed-off-by: Denis Kirjanov
> ---
> v4:
> - updated the commit and documenation
>
> v3:
> - updated the commit message
>
> v2:
> - added documentation
> - fixed padding for netif_extra_info
> ---
> ---
> xen/include/public/io/netif.h | 18 +-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/xen/include/public/io/netif.h b/xen/include/public/io/netif.h
> index 9fcf91a..a92bf04 100644
> --- a/xen/include/public/io/netif.h
> +++ b/xen/include/public/io/netif.h
> @@ -161,6 +161,17 @@
>*/
>
> /*
> + * "xdp-headroom" is used to request that extra space is added
> + * for XDP processing. The value is measured in bytes and passed by
not sure that we should use word "bytes" here as the rest of the
protocol (mostly)
talks about octets. It is somewhat mixed here, no strong opinion
> + * the frontend to be consistent between both ends.
> + * If the value is greater than zero that means that
> + * an RX response is going to be passed to an XDP program for processing.
> + *
> + * "feature-xdp-headroom" is set to "1" by the netback side like other
> features
> + * so a guest can check if an XDP program can be processed.
> + */
> +
> +/*
>* Control ring
>*
>*
> @@ -985,7 +996,8 @@ typedef struct netif_tx_request netif_tx_request_t;
> #define XEN_NETIF_EXTRA_TYPE_MCAST_ADD (2) /* u.mcast */
> #define XEN_NETIF_EXTRA_TYPE_MCAST_DEL (3) /* u.mcast */
> #define XEN_NETIF_EXTRA_TYPE_HASH (4) /* u.hash */
> -#define XEN_NETIF_EXTRA_TYPE_MAX (5)
> +#define XEN_NETIF_EXTRA_TYPE_XDP (5) /* u.xdp */
> +#define XEN_NETIF_EXTRA_TYPE_MAX (6)
>
> /* netif_extra_info_t flags. */
> #define _XEN_NETIF_EXTRA_FLAG_MORE (0)
> @@ -1018,6 +1030,10 @@ struct netif_extra_info {
> uint8_t algorithm;
> uint8_t value[4];
> } hash;
> +struct {
> +uint16_t headroom;
why do you need "pad" field here?
> +uint16_t pad[2]
> +} xdp;
> uint16_t pad[3];
> } u;
> };
Re: [PATCH 2/3] configure: also add EXTRA_PREFIX to {CPP/LD}FLAGS
On Fri, May 22, 2020 at 08:41:17AM +, Bertrand Marquis wrote: > Hi, > > As a consequence of this fix, the following has been committed (I guess as a > consequence of regenerating the configure scripts): > diff --git a/tools/configure b/tools/configure > index 375430df3f..36596389b8 100755 > --- a/tools/configure > +++ b/tools/configure > @@ -4678,6 +4678,10 @@ for ldflag in $APPEND_LIB > do > APPEND_LDFLAGS="$APPEND_LDFLAGS -L$ldflag" > done > +if ! -z $EXTRA_PREFIX ; then > +CPPFLAGS="$CPPFLAGS -I$EXTRA_PREFIX/include" > +LDFLAGS="$LDFLAGS -L$EXTRA_PREFIX/lib" > +fi > CPPFLAGS="$PREPEND_CPPFLAGS $CPPFLAGS $APPEND_CPPFLAGS" > LDFLAGS="$PREPEND_LDFLAGS $LDFLAGS $APPEND_LDFLAGS” > > This should be: > if [ ! -z $EXTRA_PREFIX ]; then > > As on other configure scripts. > > During configure I have not the following error: > ./configure: line 4681: -z: command not found > > Which is ignored but is adding -L/lib and -I/include to the CPPFLAGS and > LDFLAGS > > What should be the procedure to actually fix that (as the problem is coming > from the configure script regeneration I guess) ? Does the following patch work for you? diff --git a/m4/set_cflags_ldflags.m4 b/m4/set_cflags_ldflags.m4 index 08f5c983cc63..cd34c139bc94 100644 --- a/m4/set_cflags_ldflags.m4 +++ b/m4/set_cflags_ldflags.m4 @@ -15,7 +15,7 @@ for ldflag in $APPEND_LIB do APPEND_LDFLAGS="$APPEND_LDFLAGS -L$ldflag" done -if [ ! -z $EXTRA_PREFIX ]; then +if test ! -z $EXTRA_PREFIX ; then CPPFLAGS="$CPPFLAGS -I$EXTRA_PREFIX/include" LDFLAGS="$LDFLAGS -L$EXTRA_PREFIX/lib" fi You will need to run autogen.sh to regenerate tools/configure. Wei. > > Bertrand > > > On 5 May 2020, at 10:24, Roger Pau Monne wrote: > > > > The path provided by EXTRA_PREFIX should be added to the search path > > of the configure script, like it's done in Config.mk. Not doing so > > makes the search path for configure differ from the search path used > > by the build. > > > > Signed-off-by: Roger Pau Monné > > --- > > Please re-run autoconf.sh after applying. > > --- > > m4/set_cflags_ldflags.m4 | 4 > > 1 file changed, 4 insertions(+) > > > > diff --git a/m4/set_cflags_ldflags.m4 b/m4/set_cflags_ldflags.m4 > > index cbad3c10b0..08f5c983cc 100644 > > --- a/m4/set_cflags_ldflags.m4 > > +++ b/m4/set_cflags_ldflags.m4 > > @@ -15,6 +15,10 @@ for ldflag in $APPEND_LIB > > do > > APPEND_LDFLAGS="$APPEND_LDFLAGS -L$ldflag" > > done > > +if [ ! -z $EXTRA_PREFIX ]; then > > +CPPFLAGS="$CPPFLAGS -I$EXTRA_PREFIX/include" > > +LDFLAGS="$LDFLAGS -L$EXTRA_PREFIX/lib" > > +fi > > CPPFLAGS="$PREPEND_CPPFLAGS $CPPFLAGS $APPEND_CPPFLAGS" > > LDFLAGS="$PREPEND_LDFLAGS $LDFLAGS $APPEND_LDFLAGS"]) > > > > -- > > 2.26.2 > > > > >
Re: [PATCH 2/3] configure: also add EXTRA_PREFIX to {CPP/LD}FLAGS
Hi, As a consequence of this fix, the following has been committed (I guess as a consequence of regenerating the configure scripts): diff --git a/tools/configure b/tools/configure index 375430df3f..36596389b8 100755 --- a/tools/configure +++ b/tools/configure @@ -4678,6 +4678,10 @@ for ldflag in $APPEND_LIB do APPEND_LDFLAGS="$APPEND_LDFLAGS -L$ldflag" done +if ! -z $EXTRA_PREFIX ; then +CPPFLAGS="$CPPFLAGS -I$EXTRA_PREFIX/include" +LDFLAGS="$LDFLAGS -L$EXTRA_PREFIX/lib" +fi CPPFLAGS="$PREPEND_CPPFLAGS $CPPFLAGS $APPEND_CPPFLAGS" LDFLAGS="$PREPEND_LDFLAGS $LDFLAGS $APPEND_LDFLAGS” This should be: if [ ! -z $EXTRA_PREFIX ]; then As on other configure scripts. During configure I have not the following error: ./configure: line 4681: -z: command not found Which is ignored but is adding -L/lib and -I/include to the CPPFLAGS and LDFLAGS What should be the procedure to actually fix that (as the problem is coming from the configure script regeneration I guess) ? Bertrand > On 5 May 2020, at 10:24, Roger Pau Monne wrote: > > The path provided by EXTRA_PREFIX should be added to the search path > of the configure script, like it's done in Config.mk. Not doing so > makes the search path for configure differ from the search path used > by the build. > > Signed-off-by: Roger Pau Monné > --- > Please re-run autoconf.sh after applying. > --- > m4/set_cflags_ldflags.m4 | 4 > 1 file changed, 4 insertions(+) > > diff --git a/m4/set_cflags_ldflags.m4 b/m4/set_cflags_ldflags.m4 > index cbad3c10b0..08f5c983cc 100644 > --- a/m4/set_cflags_ldflags.m4 > +++ b/m4/set_cflags_ldflags.m4 > @@ -15,6 +15,10 @@ for ldflag in $APPEND_LIB > do > APPEND_LDFLAGS="$APPEND_LDFLAGS -L$ldflag" > done > +if [ ! -z $EXTRA_PREFIX ]; then > +CPPFLAGS="$CPPFLAGS -I$EXTRA_PREFIX/include" > +LDFLAGS="$LDFLAGS -L$EXTRA_PREFIX/lib" > +fi > CPPFLAGS="$PREPEND_CPPFLAGS $CPPFLAGS $APPEND_CPPFLAGS" > LDFLAGS="$PREPEND_LDFLAGS $LDFLAGS $APPEND_LDFLAGS"]) > > -- > 2.26.2 > >
Re: [PATCH for-4.14 2/2] tools/libxc: xc_memshr_fork with interrupts disabled
On Thu, May 21, 2020 at 03:53:23PM -0700, Tamas K Lengyel wrote: > Toolstack side for creating forks with interrupt injection disabled. > > Signed-off-by: Tamas K Lengyel Reviewed-by: Roger Pau Monné I have the same suggestion to use block instead of prohibit, so if you agree to change it on patch #1 it should also be changed here. Thanks, Roger.
Re: [PATCH for-4.14 1/2] x86/mem_sharing: Prohibit interrupt injection for forks
On Thu, May 21, 2020 at 03:53:22PM -0700, Tamas K Lengyel wrote:
> When running shallow forks without device models it may be undesirable for Xen
> to inject interrupts. With Windows forks we have observed the kernel going
> into
> infinite loops when trying to process such interrupts. By disabling interrupt
> injection the fuzzer can exercise the target code without interference.
Could you add some more information about why windows goes into
infinite loops? Is it trying to access MMIO regions of emulated
devices and getting ~0 out of them instead of the expected data and
that causes it to loop indefinitely?
> Signed-off-by: Tamas K Lengyel
> ---
> xen/arch/x86/hvm/vmx/intr.c | 4
> xen/arch/x86/mm/mem_sharing.c| 6 +-
> xen/include/asm-x86/hvm/domain.h | 2 ++
> xen/include/public/memory.h | 1 +
> 4 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/hvm/vmx/intr.c b/xen/arch/x86/hvm/vmx/intr.c
> index 000e14af49..3814795e3f 100644
> --- a/xen/arch/x86/hvm/vmx/intr.c
> +++ b/xen/arch/x86/hvm/vmx/intr.c
I think you are missing the AMD side of this change? A similar
adjustment should be done to svm_intr_assist, or else it should be
noted in the commit message the reason this is Intel only.
> @@ -256,6 +256,10 @@ void vmx_intr_assist(void)
> if ( unlikely(v->arch.vm_event) && v->arch.vm_event->sync_event )
> return;
>
> +/* Block event injection for VM fork if requested */
> +if ( unlikely(v->domain->arch.hvm.mem_sharing.prohibit_interrupts) )
> +return;
> +
> /* Crank the handle on interrupt state. */
> pt_vector = pt_update_irq(v);
>
> diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
> index 7271e5c90b..7352fce866 100644
> --- a/xen/arch/x86/mm/mem_sharing.c
> +++ b/xen/arch/x86/mm/mem_sharing.c
> @@ -2106,7 +2106,8 @@ int
> mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
> rc = -EINVAL;
> if ( mso.u.fork.pad )
> goto out;
> -if ( mso.u.fork.flags & ~XENMEM_FORK_WITH_IOMMU_ALLOWED )
> +if ( mso.u.fork.flags & ~(XENMEM_FORK_WITH_IOMMU_ALLOWED |
> + XENMEM_FORK_PROHIBIT_INTERRUPTS) )
Nit: I would move the XENMEM_FORK_ option ORing to a newline, so that
you don't have to use a whole line every time a new option is used.
Ie:
if ( mso.u.fork.flags &
~(XENMEM_FORK_WITH_IOMMU_ALLOWED | XENMEM_FORK_BLOCK_INTERRUPTS) )
> goto out;
>
> rc = rcu_lock_live_remote_domain_by_id(mso.u.fork.parent_domain,
> @@ -2134,6 +2135,9 @@ int
> mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
> rc = hypercall_create_continuation(__HYPERVISOR_memory_op,
> "lh", XENMEM_sharing_op,
> arg);
> +else if ( !rc && (mso.u.fork.flags &
> XENMEM_FORK_PROHIBIT_INTERRUPTS) )
> +d->arch.hvm.mem_sharing.prohibit_interrupts = true;
> +
> rcu_unlock_domain(pd);
> break;
> }
> diff --git a/xen/include/asm-x86/hvm/domain.h
> b/xen/include/asm-x86/hvm/domain.h
> index 95fe18cddc..e114f818d3 100644
> --- a/xen/include/asm-x86/hvm/domain.h
> +++ b/xen/include/asm-x86/hvm/domain.h
> @@ -74,6 +74,8 @@ struct mem_sharing_domain
> * to resume the search.
> */
> unsigned long next_shared_gfn_to_relinquish;
> +
> +bool prohibit_interrupts;
Nit: I would prefer block_interrupts, prohibit seems very formal to
me, but I'm not a native speaker, so feel free to ignore this
suggestion.
> };
> #endif
>
> diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
> index dbd35305df..fe2e6caa68 100644
> --- a/xen/include/public/memory.h
> +++ b/xen/include/public/memory.h
> @@ -537,6 +537,7 @@ struct xen_mem_sharing_op {
> struct mem_sharing_op_fork { /* OP_FORK */
> domid_t parent_domain;/* IN: parent's domain id */
> #define XENMEM_FORK_WITH_IOMMU_ALLOWED (1u << 0)
> +#define XENMEM_FORK_PROHIBIT_INTERRUPTS (1u << 1)
FWIW, I would also use BLOCK here instead of PROHIBIT.
Thanks, Roger.
Re: [PATCH] xen/events: avoid NULL pointer dereference in evtchn_from_irq()
On 21.05.20 23:57, Boris Ostrovsky wrote: On 5/21/20 2:46 PM, Marek Marczykowski-Górecki wrote: On Thu, May 21, 2020 at 01:22:03PM -0400, Boris Ostrovsky wrote: On 3/19/20 3:14 AM, Juergen Gross wrote: There have been reports of races in evtchn_from_irq() where the info pointer has been NULL. Avoid that case by testing info before dereferencing it. In order to avoid accessing a just freed info structure do the kfree() via kfree_rcu(). Looks like noone ever responded to this. This change looks fine but is there a background on the problem? I looked in the archives and didn't find the relevant discussion. Here is the original bug report: https://xen.markmail.org/thread/44apwkwzeme4uavo Thanks. Do we know what the race is? Is it because an event is being delivered from a dying guest who is in the process of tearing down event channels? Missing synchronization between event channel (de-)allocation and handling. I have a patch sitting here, just didn't have the time to do some proper testing and sending it out. Juergen
[PATCH] x86/idle: prevent entering C3/C6 on some Intel CPUs due to errata
Apply a workaround for errata BA80, AAK120, AAM108, AAO67, BD59,
AAY54: Rapid Core C3/C6 Transition May Cause Unpredictable System
Behavior.
Limit maximum C state to C2 when SMT is enabled on the affected CPUs.
Signed-off-by: Roger Pau Monné
---
xen/arch/x86/cpu/intel.c | 37 +
1 file changed, 37 insertions(+)
diff --git a/xen/arch/x86/cpu/intel.c b/xen/arch/x86/cpu/intel.c
index b77c1a78ed..69e99bb358 100644
--- a/xen/arch/x86/cpu/intel.c
+++ b/xen/arch/x86/cpu/intel.c
@@ -296,6 +296,41 @@ static void early_init_intel(struct cpuinfo_x86 *c)
ctxt_switch_levelling(NULL);
}
+/*
+ * Errata BA80, AAK120, AAM108, AAO67, BD59, AAY54: Rapid Core C3/C6 Transition
+ * May Cause Unpredictable System Behavior
+ *
+ * Under a complex set of internal conditions, cores rapidly performing C3/C6
+ * transitions in a system with Intel Hyper-Threading Technology enabled may
+ * cause a machine check error (IA32_MCi_STATUS.MCACOD = 0x0106), system hang
+ * or unpredictable system behavior.
+ */
+static void probe_c3_errata(const struct cpuinfo_x86 *c)
+{
+#define INTEL_FAM6_MODEL(m) { X86_VENDOR_INTEL, 6, m, X86_FEATURE_ALWAYS }
+static const struct x86_cpu_id models[] = {
+/* Nehalem */
+INTEL_FAM6_MODEL(0x1a),
+INTEL_FAM6_MODEL(0x1e),
+INTEL_FAM6_MODEL(0x1f),
+INTEL_FAM6_MODEL(0x2e),
+/* Westmere (note Westmere-EX is not affected) */
+INTEL_FAM6_MODEL(0x2c),
+INTEL_FAM6_MODEL(0x25),
+{ }
+};
+#undef INTEL_FAM6_MODEL
+
+/* Serialized by the AP bringup code. */
+if ( max_cstate > 1 && (c->apicid & (c->x86_num_siblings - 1)) &&
+ x86_match_cpu(models) )
+{
+printk(XENLOG_WARNING
+ "Disabling C-states C3 and C6 due to CPU errata\n");
+max_cstate = 1;
+}
+}
+
/*
* P4 Xeon errata 037 workaround.
* Hardware prefetcher may cause stale data to be loaded into the cache.
@@ -323,6 +358,8 @@ static void Intel_errata_workarounds(struct cpuinfo_x86 *c)
if (cpu_has_tsx_force_abort && opt_rtm_abort)
wrmsrl(MSR_TSX_FORCE_ABORT, TSX_FORCE_ABORT_RTM);
+
+ probe_c3_errata(c);
}
--
2.26.2
[qemu-mainline test] 150312: tolerable FAIL - PUSHED
flight 150312 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/150312/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10 fail blocked in 150271 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 150271 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 150271 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 150271 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 150271 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 150271 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: qemuuae3aa5da96f4ccf0c2a28851449d92db9fcfad71 baseline version: qemuuf2465433b43fb87766d79f42191607dac4aed5b4 Last test of basis 150271 2020-05-20 06:14:37 Z2 days Failing since150297 2020-05-21 10:37:07 Z0 days2 attempts Testing same since 150312 2020-05-21 23:10:46 Z0 days1 attempts People who touched revisions under test: Alex Bennée Daniel P. Berrangé Eric Blake Gerd Hoffmann John Snow Peter Maydell Richard Henderson Volker Rümelin xiaoqiang zhao jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64
