Re: [Xen-devel] [PATCH v3 08/16] x86/hvm: rearrange content of hvm.h
>>> On 04.09.18 at 18:15, wrote: > Move enum and function declarations to first half of the file. > > Static inline functions and macros, which reference HVM specific > fields directly are grouped together in second half of the file. > > The movement is needed because in a later patch the second half is > going to be enclosed in CONFIG_HVM. > > Pure code movement. No functional change. > > Signed-off-by: Wei Liu Acked-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/balloon: add runtime control for scrubbing ballooned out pages
>>> On 06.09.18 at 17:33, wrote:
> --- a/include/xen/mem-reservation.h
> +++ b/include/xen/mem-reservation.h
> @@ -17,12 +17,17 @@
>
> #include
>
> +#ifdef CONFIG_XEN_SCRUB_PAGES
> +extern bool xen_scrub_pages;
> +
> static inline void xenmem_reservation_scrub_page(struct page *page)
> {
> -#ifdef CONFIG_XEN_SCRUB_PAGES
> - clear_highpage(page);
> -#endif
> + if (xen_scrub_pages)
> + clear_highpage(page);
> }
> +#else
> +static inline void xenmem_reservation_scrub_page(struct page *page) { }
> +#endif
Wouldn't CONFIG_XEN_SCRUB_PAGES then better become
CONFIG_XEN_SCRUB_PAGES_DEFAULT, with the value used as
initializer of xen_scrub_pages?
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-linus bisection] complete test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow
branch xen-unstable xenbranch xen-unstable job test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow testid debian-hvm-install Tree: linux git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: xen git://xenbits.xen.org/xen.git *** Found and reproduced problem changeset *** Bug is in tree: xen git://xenbits.xen.org/xen.git Bug introduced: 580c458699e367bf427967844fa79086b60da675 Bug not present: 6425f91c72525295a551bf148d9a6b0fa7971097 Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/127362/ commit 580c458699e367bf427967844fa79086b60da675 Author: Andrew Cooper Date: Mon Mar 19 16:50:46 2018 + xen/domain: Call arch_domain_create() as early as possible in domain_create() This is in preparation to set up d->max_cpus and d->vcpu[] in domain_create(), and allow later parts of domain construction to have access to the values. Signed-off-by: Andrew Cooper Reviewed-by: Roger Pau Monné Reviewed-by: Jan Beulich For bisection revision-tuple graph see: http://logs.test-lab.xenproject.org/osstest/results/bisect/linux-linus/test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow.debian-hvm-install.html Revision IDs in each graph node refer, respectively, to the Trees above. Running cs-bisection-step --graph-out=/home/logs/results/bisect/linux-linus/test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow.debian-hvm-install --summary-out=tmp/127362.bisection-summary --basis-template=125898 --blessings=real,real-bisect linux-linus test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow debian-hvm-install Searching for failure / basis pass: 127344 fail [host=godello1] / 126310 [host=albana1] 126202 [host=albana0] 126069 [host=baroque1] 125921 [host=chardonnay0] 125898 [host=chardonnay1] 125702 [host=rimava1] 125676 [host=baroque1] 125657 [host=albana0] 125648 [host=albana1] 125639 [host=italia0] 125585 ok. Failure / basis pass flights: 127344 / 125585 (tree with no url: minios) (tree with no url: ovmf) (tree with no url: seabios) Tree: linux git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: xen git://xenbits.xen.org/xen.git Latest b36fdc6853a38a6f8749897a33435635019e0647 c530a75c1e6a472b0eb9558310b518f0dfcd8860 9c0eed618f37dd5b4a57c8b3fbc48ef8913e3149 de5b678ca4dcdfa83e322491d478d66df56c1986 da3bd8111858a1fb045a6ddc0b36d72164d9c5dd Basis pass 6e77b267723c723d4717abc874e00059ab07a46a c530a75c1e6a472b0eb9558310b518f0dfcd8860 c8ea0457495342c417c3dc033bba25148b279f60 43139135a8938de44f66333831d3a8655d07663a e3f667bc5f51d0aa44357a64ca134cd952679c81 Generating revisions with ./adhoc-revtuple-generator git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git#6e77b267723c723d4717abc874e00059ab07a46a-b36fdc6853a38a6f8749897a33435635019e0647 git://xenbits.xen.org/osstest/linux-firmware.git#c530a75c1e6a472b0eb9558310b518f0dfcd8860-c530a75c1e6a472b0eb9558310b518f0dfcd8860 git://xenbits.xen.org/qemu-xen-traditional.git#c8ea0457495342c417c3dc033bba25148b279f60-9c0eed618f37dd5b4a57c8b3fbc48ef8913e3149 git://xenbits.xen.org/qemu-xen.git#43139135a8938de44f66333831d3a8655d07663a-de5b678ca4dcdfa83e322491d478d66df56c1986 git://xenbits.xen.org/xen.git#e3f667bc5f51d0aa44357a64ca134cd952679c81-da3bd8111858a1fb045a6ddc0b36d72164d9c5dd adhoc-revtuple-generator: tree discontiguous: linux-2.6 adhoc-revtuple-generator: tree discontiguous: qemu-xen Loaded 2006 nodes in revision graph Searching for test results: 125167 [host=rimava1] 125242 [host=debina0] 125285 [host=huxelrebe1] 125401 [host=joubertin1] 125501 [host=pinot0] 125551 [host=chardonnay1] 125520 [host=fiano1] 125585 pass 6e77b267723c723d4717abc874e00059ab07a46a c530a75c1e6a472b0eb9558310b518f0dfcd8860 c8ea0457495342c417c3dc033bba25148b279f60 43139135a8938de44f66333831d3a8655d07663a e3f667bc5f51d0aa44357a64ca134cd952679c81 125648 [host=albana1] 125639 [host=italia0] 125657 [host=albana0] 125676 [host=baroque1] 125702 [host=rimava1] 125898 [host=chardonnay1] 125921 [host=chardonnay0] 126069 [host=baroque1] 126202 [host=albana0] 126310 [host=albana1] 126412 fail irrelevant 126550 fail irrelevant 126682 fail irrelevant 126888 fail irrelevant 126978 fail irrelevant 127038 fail irrelevant 127108 fail irrelevant 127148 fail irrelevant 127193 fail irrelevant 127221 fail irrelevant 127256 fail irrelevant 127342 pass 6e77b267723c723d4717abc874e00059ab07a46a c530a75c1e6a472b0eb9558310b518f0dfcd8860 9c0eed618f37dd5b4a57c8b3fbc48ef8913e3149 43139135a8938de44f66333831d3a8655d07663a e5d6ddcd31a6113e4a3db7a235ca78770fe8f401
[Xen-devel] [PATCH v3] xen: avoid crash in disable_hotplug_cpu
The command 'xl vcpu-set 0 0', issued in dom0, will crash dom0:
BUG: unable to handle kernel NULL pointer dereference at 02d8
PGD 0 P4D 0
Oops: [#1] PREEMPT SMP NOPTI
CPU: 7 PID: 65 Comm: xenwatch Not tainted 4.19.0-rc2-1.ga9462db-default #1
openSUSE Tumbleweed (unreleased)
Hardware name: Intel Corporation S5520UR/S5520UR, BIOS
S5500.86B.01.00.0050.050620101605 05/06/2010
RIP: e030:device_offline+0x9/0xb0
Code: 77 24 00 e9 ce fe ff ff 48 8b 13 e9 68 ff ff ff 48 8b 13 e9 29 ff ff ff
48 8b 13 e9 ea fe ff ff 90 66 66 66 66 90 41 54 55 53 87 d8 02 00 00 01 0f
85 88 00 00 00 48 c7 c2 20 09 60 81 31 f6
RSP: e02b:c90040f27e80 EFLAGS: 00010203
RAX: RBX: RCX:
RDX: 8801f380 RSI: c90040f27e70 RDI:
RBP: R08: 820e47b3 R09:
R10: 7ff0 R11: R12: 822e6d30
R13: dead0200 R14: dead0100 R15: 8158b4e0
FS: 7ffa595158c0() GS:8801f39c() knlGS:
CS: e033 DS: ES: CR0: 80050033
CR2: 02d8 CR3: 0001d9602000 CR4: 2660
Call Trace:
handle_vcpu_hotplug_event+0xb5/0xc0
xenwatch_thread+0x80/0x140
? wait_woken+0x80/0x80
kthread+0x112/0x130
? kthread_create_worker_on_cpu+0x40/0x40
ret_from_fork+0x3a/0x50
This happens because handle_vcpu_hotplug_event is called twice. In the
first iteration cpu_present is still true, in the second iteration
cpu_present is false which causes get_cpu_device to return NULL.
In case of cpu#0, cpu_online is apparently always true.
Fix this crash by checking if the cpu can be hotplugged, which is false
for a cpu that was just removed.
Also check if the cpu was actually offlined by device_remove, otherwise
leave the cpu_present state as it is.
Signed-off-by: Olaf Hering
---
drivers/xen/cpu_hotplug.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/xen/cpu_hotplug.c b/drivers/xen/cpu_hotplug.c
index d4265c8ebb22..68f9f663da08 100644
--- a/drivers/xen/cpu_hotplug.c
+++ b/drivers/xen/cpu_hotplug.c
@@ -19,11 +19,15 @@ static void enable_hotplug_cpu(int cpu)
static void disable_hotplug_cpu(int cpu)
{
+ if (!cpu_is_hotpluggable(cpu))
+ return;
if (cpu_online(cpu)) {
lock_device_hotplug();
device_offline(get_cpu_device(cpu));
unlock_device_hotplug();
}
+ if (cpu_online(cpu))
+ return;
if (cpu_present(cpu))
xen_arch_unregister_cpu(cpu);
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/5] x86/hvm: Switch hvm_allow_get_param() to use a whitelist
>>> On 06.09.18 at 17:21, wrote:
> On 06/09/18 09:56, Paul Durrant wrote:
>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>>> index c22bf0b..96a6323 100644
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -4350,7 +4350,7 @@ static int hvm_allow_get_param(struct domain *d,
>>>
>>> switch ( a->index )
>>> {
>>> -/* The following parameters can be read by the guest. */
>>> +/* The following parameters can be read by the guest and
>>> toolstack. */
>> Intentional indentation change? I guess so since you do it again below, but
>> just checking.
>
> Yes - this is where BSD style puts comments, as you are inside the
> braced scope of the switch statement.
Well, I'm inclined to support Paul here, no matter what BSD style may
have to say. Our own style description doesn't spell out anything
either way...
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v6 01/14] iommu: introduce the concept of BFN...
>>> On 07.09.18 at 03:47, wrote: >> From: Paul Durrant [mailto:paul.durr...@citrix.com] >> Sent: Thursday, September 6, 2018 10:54 PM >> >> > -Original Message- >> > From: Jan Beulich [mailto:jbeul...@suse.com] >> > Sent: 06 September 2018 14:13 >> > To: Paul Durrant >> > Cc: Suravee Suthikulpanit ; Julien Grall >> > ; Kevin Tian ; Stefano >> > Stabellini ; xen-devel > > de...@lists.xenproject.org> >> > Subject: RE: [Xen-devel] [PATCH v6 01/14] iommu: introduce the concept >> of >> > BFN... >> > >> > >>> On 06.09.18 at 12:36, wrote: >> > >> From: Jan Beulich [mailto:jbeul...@suse.com] >> > >> Sent: 05 September 2018 10:39 >> > >> >> > >> >>> On 05.09.18 at 11:13, wrote: >> > >> > Personally I think 'bus address' is commonly enough used term for >> > >> addresses >> > >> > used by devices for DMA. Indeed we have already 'dev_bus_addr' in >> > the >> > >> grant >> > >> > map and unmap hypercalls. So baddr and bfn seem like ok terms to >> me. >> > It's >> > >> > also not impossible to rename these later if they prove problematic. >> > >> >> > >> But that's the point - the names are problematic (to me): I >> permanently >> > >> have to remind myself that they do _not_ refer to the addresses as >> > >> seen when accessing memory, but the ones going _into_ the IOMMU. >> > > >> > > Ok. Could we agree on 'IOFN' then? I think 'iova' and 'io address' are >> also >> > > reasonably widely used terms to refer to address from a device's PoV. >> I'd >> > > really like to unblock these early patches. >> > >> > Hmm, earlier I had indicated I'd prefer DFN (as this make clear whose >> > view we are talking about). Kevin seemed to prefer DFN too, just with >> > a different association for D (which, as said, I consider unhelpful). So >> > is there a particular reason you're now suggesting IOFN nevertheless? >> >> It was the ambiguity and lack of agreement over the 'D' that made me think >> that the other alternative would be better. >> Kevin, would you be ok with 'IOFN'? >> > > My problem with DFN is when combining D with address then "device > address" is not very clear to me while interpreting D as DMA is also > not that clear from Jan's point. What about making its description mention both possible interpretations? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen: avoid crash in disable_hotplug_cpu
Am Thu, 6 Sep 2018 22:31:45 +0200 schrieb Olaf Hering : > IF cpu0_hotpluggable is broken, then only "if (!cpu) return;" can help. Another option is to do cpu_online() twice to check if device_offline did anything. Not sure if the compiler would fold the two checks into a single check. Olaf pgp8PgPrRisKv.pgp Description: Digitale Signatur von OpenPGP ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [ovmf test] 127357: all pass - PUSHED
flight 127357 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/127357/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 7761cee050e706544bfee70f1c6ca894be0b1a2a baseline version: ovmf 7bf1eb6ef8ae6ccffca6fc8f36b132be704c0b1b Last test of basis 127354 2018-09-07 01:10:55 Z0 days Testing same since 127357 2018-09-07 03:41:59 Z0 days1 attempts People who touched revisions under test: Carsey, Jaben Jaben Carsey 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 7bf1eb6ef8..7761cee050 7761cee050e706544bfee70f1c6ca894be0b1a2a -> xen-tested-master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen: avoid crash in disable_hotplug_cpu
On 06/09/18 22:31, Olaf Hering wrote: > Am Thu, 6 Sep 2018 14:45:57 -0400 > schrieb Boris Ostrovsky : > >> On 09/06/2018 02:37 AM, Olaf Hering wrote: >>> The command 'xl vcpu-set 0 0', issued in dom0, will crash dom0: > >>> This happens because handle_vcpu_hotplug_event is called twice. In the >>> first iteration cpu_present is still true, in the second iteration >>> cpu_present is false which causes get_cpu_device to return NULL. >>> In case of cpu#0, cpu_online is apparently always true. > >> I think we should check both this and num_online_cpus() != 0. > > This can not possibly help. cpu#0 is the first one that goes offline. > IF cpu0_hotpluggable is broken, then only "if (!cpu) return;" can help. We should add the Xen PV guest test to arch_register_cpu() and switch cpu0_hotpluggable off. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [ovmf test] 127354: all pass - PUSHED
flight 127354 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/127354/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 7bf1eb6ef8ae6ccffca6fc8f36b132be704c0b1b baseline version: ovmf b48ec0e8ab1c881ea584ed76d2da0bac09db38ef Last test of basis 127346 2018-09-06 17:42:20 Z0 days Testing same since 127354 2018-09-07 01:10:55 Z0 days1 attempts People who touched revisions under test: Carsey, Jaben Jaben Carsey 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 b48ec0e8ab..7bf1eb6ef8 7bf1eb6ef8ae6ccffca6fc8f36b132be704c0b1b -> xen-tested-master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] x86 Community Call - Wed Sept 12, 14:00 - 15:00 UTC - Call for agenda items
> On Sep 6, 2018, at 11:35, Lars Kurth wrote: > > Dear community members, > > please send me agenda items for next week’s community call by next > Monday. "Argo" (formerly v4v) inter-VM communication mechanism for Xen 4.12, discussed at Xen Summit 2018 design session and PSEC 2018, design doc forthcoming. Variants of this technology have been used in OpenXT and uXen/Bromium for several years. Video: https://www.platformsecuritysummit.com/2018/speaker/clark/ > One suggestion which was made to me recently by Rich, > Christopher and Daniel was for Daniel to give an overview of > Secure Boot, Measured Launch and TrenchBoot. > > @Daniel, @Christopher, @Rich: would this be possible during the > next call? How much time does the talk require. If attendees can watch Daniel Smith’s TrenchBoot video as a pre-requisite for the call, his overview will build on the video: 15 min talk + 15 mins of discussion. Video: https://www.platformsecuritysummit.com/2018/speaker/smith/ There were boot integrity talks on Xen and/or UEFI, from AIS, Dell, Intel, Oracle: https://www.platformsecuritysummit.com/2018/topic/boot/ Rich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v6 01/14] iommu: introduce the concept of BFN...
> From: Paul Durrant [mailto:paul.durr...@citrix.com] > Sent: Thursday, September 6, 2018 10:54 PM > > > -Original Message- > > From: Jan Beulich [mailto:jbeul...@suse.com] > > Sent: 06 September 2018 14:13 > > To: Paul Durrant > > Cc: Suravee Suthikulpanit ; Julien Grall > > ; Kevin Tian ; Stefano > > Stabellini ; xen-devel > de...@lists.xenproject.org> > > Subject: RE: [Xen-devel] [PATCH v6 01/14] iommu: introduce the concept > of > > BFN... > > > > >>> On 06.09.18 at 12:36, wrote: > > >> From: Jan Beulich [mailto:jbeul...@suse.com] > > >> Sent: 05 September 2018 10:39 > > >> > > >> >>> On 05.09.18 at 11:13, wrote: > > >> > Personally I think 'bus address' is commonly enough used term for > > >> addresses > > >> > used by devices for DMA. Indeed we have already 'dev_bus_addr' in > > the > > >> grant > > >> > map and unmap hypercalls. So baddr and bfn seem like ok terms to > me. > > It's > > >> > also not impossible to rename these later if they prove problematic. > > >> > > >> But that's the point - the names are problematic (to me): I > permanently > > >> have to remind myself that they do _not_ refer to the addresses as > > >> seen when accessing memory, but the ones going _into_ the IOMMU. > > > > > > Ok. Could we agree on 'IOFN' then? I think 'iova' and 'io address' are > also > > > reasonably widely used terms to refer to address from a device's PoV. > I'd > > > really like to unblock these early patches. > > > > Hmm, earlier I had indicated I'd prefer DFN (as this make clear whose > > view we are talking about). Kevin seemed to prefer DFN too, just with > > a different association for D (which, as said, I consider unhelpful). So > > is there a particular reason you're now suggesting IOFN nevertheless? > > It was the ambiguity and lack of agreement over the 'D' that made me think > that the other alternative would be better. > Kevin, would you be ok with 'IOFN'? > My problem with DFN is when combining D with address then "device address" is not very clear to me while interpreting D as DMA is also not that clear from Jan's point. I didn't see a perfect candidate without causing any ambiguity - at this point I feel IOFN/IOVA might not be the best one but it wins to me when considering the fact that it is widely used term in other places (e.g. in vtd spec, in Linux vfio/iommu driver, etc.). Thanks Kevin ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [ovmf test] 127346: all pass - PUSHED
flight 127346 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/127346/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf b48ec0e8ab1c881ea584ed76d2da0bac09db38ef baseline version: ovmf 22cf747fcf75dbfe51e5524ce1f9cf17b19914cd Last test of basis 127335 2018-09-06 09:51:21 Z0 days Testing same since 127346 2018-09-06 17:42:20 Z0 days1 attempts People who touched revisions under test: Laszlo Ersek 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 22cf747fcf..b48ec0e8ab b48ec0e8ab1c881ea584ed76d2da0bac09db38ef -> xen-tested-master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/3] xen/vcpu: Rework sanity checks in vcpu_create()
On 06/09/18 21:07, Jason Andryuk wrote: > On Thu, Sep 6, 2018 at 3:28 PM Andrew Cooper > wrote: >> Poisoning idle_vcpu[0] with the sanity debug value isn't actually a clever >> idea, because it passes a NULL pointer check but isn't a usable vcpu. It is >> also the reason for the (!is_idle_domain(d) || vcpu_id) part of the existing >> sanity BUG_ON(). >> >> Now that d->max_vcpus is appropriately set up before vcpu_create() is called, >> we can properly range check the requested vcpu_id. Drop the BUG_ON() and >> replace it with code which is runtime safe but non-fatal. >> >> While v0 must be the first allocated vcpu for for_each_vcpu() to work, it >> isn't a requirement for the threading the vcpu into the linked list, so >> update >> the threading code to be more generic, and add a comment explaining why we >> need to search for prev_id. >> >> Signed-off-by: Andrew Cooper >> --- >> CC: Jan Beulich >> CC: Wei Liu >> CC: Roger Pau Monné >> CC: Stefano Stabellini >> CC: Julien Grall >> --- >> xen/arch/arm/setup.c | 1 - >> xen/arch/x86/setup.c | 1 - >> xen/common/domain.c | 32 >> 3 files changed, 28 insertions(+), 6 deletions(-) >> >> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c >> index 01b..d06ac40 100644 >> --- a/xen/arch/arm/setup.c >> +++ b/xen/arch/arm/setup.c >> @@ -704,7 +704,6 @@ void __init start_xen(unsigned long boot_phys_offset, >> set_processor_id(0); /* needed early, for smp_processor_id() */ >> >> set_current((struct vcpu *)0xf000); /* debug sanity */ >> -idle_vcpu[0] = current; >> >> setup_virtual_regions(NULL, NULL); >> /* Initialize traps early allow us to get backtrace when an error >> occurred */ >> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c >> index a2f22a1..5e1e8ae 100644 >> --- a/xen/arch/x86/setup.c >> +++ b/xen/arch/x86/setup.c >> @@ -691,7 +691,6 @@ void __init noreturn __start_xen(unsigned long mbi_p) >> >> set_processor_id(0); >> set_current(INVALID_VCPU); /* debug sanity. */ >> -idle_vcpu[0] = current; > xen/arch/x86/tboot.c:tboot_shutdown() has: > if ( idle_vcpu[0] != INVALID_VCPU ) > write_ptbase(idle_vcpu[0]); > > With your change, this should be update to check for non-NULL. Oh - very good catch. Thanks. Looking at the code, ideally it would change to write_cr3(idle_pg_table), but that isn't going to include the additional CR4 changes. Leaving this in this form (with a NULL) check is probably best. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen: avoid crash in disable_hotplug_cpu
On 09/06/2018 04:31 PM, Olaf Hering wrote: > Am Thu, 6 Sep 2018 14:45:57 -0400 > schrieb Boris Ostrovsky : > >> On 09/06/2018 02:37 AM, Olaf Hering wrote: >>> The command 'xl vcpu-set 0 0', issued in dom0, will crash dom0: >>> This happens because handle_vcpu_hotplug_event is called twice. In the >>> first iteration cpu_present is still true, in the second iteration >>> cpu_present is false which causes get_cpu_device to return NULL. >>> In case of cpu#0, cpu_online is apparently always true. >> I think we should check both this and num_online_cpus() != 0. > This can not possibly help. cpu#0 is the first one that goes offline. > IF cpu0_hotpluggable is broken, then only "if (!cpu) return;" can help. And maybe that needs to be part of the check, in addition to cpu_is_hotpluggable() test. Offlining CPU0 is problematic. For example, look at xen_pv_cpu_disable(). -boris signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
On 2018/08/27 16:41, Christian König wrote:
> Am 26.08.2018 um 10:40 schrieb Tetsuo Handa:
>> I'm not following. Why don't we need to do like below (given that
>> nobody except amdgpu_mn_read_lock() holds ->read_lock) because e.g.
>> drm_sched_fence_create() from drm_sched_job_init() from amdgpu_cs_submit()
>> is doing GFP_KERNEL memory allocation with ->lock held for write?
>
> That's a bug which needs to be fixed separately.
>
> Allocating memory with GFP_KERNEL while holding a lock which is also taken in
> the reclaim code path is illegal not matter what you do.
>
> Patches to fix this are already on the appropriate mailing list and will be
> pushed upstream today.
>
> Regards,
> Christian.
Commit 4a2de54dc1d7668f ("drm/amdgpu: fix holding mn_lock while allocating
memory")
seems to be calling amdgpu_mn_unlock() without amdgpu_mn_lock() when
drm_sched_job_init() failed...
Michal, you are asking me to fix all bugs (including out of tree code) and
prevent
future bugs just because you want to avoid using timeout in order to avoid OOM
lockup
( https://marc.info/?i=55a3fb37-3246-73d7-0f45-5835a3f48...@i-love.sakura.ne.jp
).
That is a too much request which is impossible for even you. More you count on
the OOM reaper, we exponentially complicates dependency and more likely to
stumble
over unreviewed/untested code...
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen-blkback: Switch to closed state after releasing the backing device
On Thu, Sep 06, 2018 at 06:29:32PM +0200, Roger Pau Monné wrote:
> On Wed, Sep 05, 2018 at 06:28:01PM +0200, Valentin Vidic wrote:
> > On Wed, Sep 05, 2018 at 01:35:15PM +0200, Valentin Vidic wrote:
> > > > AFAICT, this will cause the backend to never switch to 'Closed' state
> > > > until the toolstack sets online to 0, which is not good IMO.
> > > >
> > > > If for example a frontend decides to close a device, the backend will
> > > > stay in state 'Closing' until the toolstack actually removes the disk
> > > > by setting online to 0.
> > > >
> > > > This will prevent resetting blk connections, as blkback will refuse to
> > > > switch to state XenbusStateInitWait unless it's at XenbusStateClosed
> > > > (see the XenbusStateInitialising case in frontend_changed), which will
> > > > never be reached with your patch.
> >
> > Would it be possible to call xen_vbd_free before the state change?
> >
> > case XenbusStateClosed:
> > xen_blkif_disconnect(be->blkif);
> > xen_vbd_free(&be->blkif->vbd);
> > xenbus_switch_state(dev, XenbusStateClosed);
>
> I think that will break reconnection, since xen_vbd_create is only
> called after hotplug script execution is performed (which happens only
> once at device attachment), but not when DomU changes frontend
> state.
>
> If you want to perform this xen_vbd_free you will also have to move
> the xen_vbd_create call AFAICT, to a place that's also called when
> reconnecting a device. Note that I could be wrong, so it might be
> worth a shot to try different approaches since the blkback code is
> quite tangled and I might miss something.
It seems like the Closed state is not a good point to call the remove
script since the device could go back from Closed to Connected.
Maybe it would help to introduce a new final state (7 = XenbusStateFree
or XenbusStateRemove) that would be set after xen_vbd_free to let the
userspace know it is safe to run the remove script?
static void xen_blkif_free(struct xen_blkif *blkif)
{
WARN_ON(xen_blkif_disconnect(blkif));
xen_vbd_free(&blkif->vbd);
xenbus_switch_state(blkif->be->dev, XenbusStateFree);
kfree(blkif->be->mode);
kfree(blkif->be);
/* Make sure everything is drained before shutting down */
kmem_cache_free(xen_blkif_cachep, blkif);
}
--
Valentin
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen-blkback: Switch to closed state after releasing the backing device
On Thu, Sep 06, 2018 at 06:14:21PM +0200, Roger Pau Monné wrote:
> On Wed, Sep 05, 2018 at 06:27:56PM +0200, Valentin Vidic wrote:
> > On Wed, Sep 05, 2018 at 12:36:49PM +0200, Roger Pau Monné wrote:
> > > On Wed, Aug 29, 2018 at 08:52:14AM +0200, Valentin Vidic wrote:
> > > > Switching to closed state earlier can cause the block-drbd
> > > > script to fail with 'Device is held open by someone':
> > > >
> > > > root: /etc/xen/scripts/block-drbd: remove
> > > > XENBUS_PATH=backend/vbd/6/51712
> > > > kernel: [ .278235] block drbd6: State change failed: Device is held
> > > > open by someone
> > > > kernel: [ .278304] block drbd6: state = { cs:Connected
> > > > ro:Primary/Secondary ds:UpToDate/UpToDate r- }
> > > > kernel: [ .278340] block drbd6: wanted = { cs:Connected
> > > > ro:Secondary/Secondary ds:UpToDate/UpToDate r- }
> > > > root: /etc/xen/scripts/block-drbd: Writing
> > > > backend/vbd/6/51712/hotplug-error /etc/xen/scripts/block-drbd failed;
> > > > error detected. backend/vbd/6/51712/hotplug-status error to xenstore.
> > > > root: /etc/xen/scripts/block-drbd: /etc/xen/scripts/block-drbd failed;
> > > > error detected.
> > > >
> > > > Signed-off-by: Valentin Vidic
> > > > Cc: sta...@vger.kernel.org
> > > > ---
> > > > drivers/block/xen-blkback/xenbus.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/block/xen-blkback/xenbus.c
> > > > b/drivers/block/xen-blkback/xenbus.c
> > > > index a4bc74e72c39..43bddc996709 100644
> > > > --- a/drivers/block/xen-blkback/xenbus.c
> > > > +++ b/drivers/block/xen-blkback/xenbus.c
> > > > @@ -323,6 +323,7 @@ static void xen_blkif_free(struct xen_blkif *blkif)
> > > > {
> > > > WARN_ON(xen_blkif_disconnect(blkif));
> > > > xen_vbd_free(&blkif->vbd);
> > > > + xenbus_switch_state(blkif->be->dev, XenbusStateClosed);
> > > > kfree(blkif->be->mode);
> > > > kfree(blkif->be);
> > > >
> > > > @@ -814,7 +815,6 @@ static void frontend_changed(struct xenbus_device
> > > > *dev,
> > > >
> > > > case XenbusStateClosed:
> > > > xen_blkif_disconnect(be->blkif);
> > > > - xenbus_switch_state(dev, XenbusStateClosed);
> > > > if (xenbus_dev_is_online(dev))
> > > > break;
> > >
> > > AFAICT, this will cause the backend to never switch to 'Closed' state
> > > until the toolstack sets online to 0, which is not good IMO.
> > >
> > > If for example a frontend decides to close a device, the backend will
> > > stay in state 'Closing' until the toolstack actually removes the disk
> > > by setting online to 0.
> > >
> > > This will prevent resetting blk connections, as blkback will refuse to
> > > switch to state XenbusStateInitWait unless it's at XenbusStateClosed
> > > (see the XenbusStateInitialising case in frontend_changed), which will
> > > never be reached with your patch.
> >
> > Ok, is it possible to test this somehow?
>
> Yes, you can try booting a PV guest with pvgrub, that will cause
> pvgrub to open a blk connection, then close it and afterwards the OS
> kernel will start a new connection.
Indeed the boot hangs after pvgrub loads the kernel and initrd, so it
seems this does not work as expected.
--
Valentin
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable test] 127327: regressions - trouble: broken/fail/pass
flight 127327 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/127327/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xen-freebsd broken build-amd64-xen-freebsd 5 host-install(5)broken REGR. vs. 127301 test-armhf-armhf-examine 8 reboot fail REGR. vs. 127280 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 127280 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 127280 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 127301 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 127301 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 127301 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 127301 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 127301 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 127301 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 127301 build-amd64-xen-xsm-freebsd 7 xen-build-freebsdfail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 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-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-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-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-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-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-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail 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-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass version targeted for testing: xen 2c0b1824b1cb33a2610f3f55299247f9e0464466 baseline version: xen da3bd8111858a1fb045a6ddc0b36d72164d9c5dd Last test of basis 127301 2018-09-05 08:15:56 Z1 days Testing same since 127327 2018-09-06 03:41:22 Z0 days1 attempts People who touched revisions under test: Boris Ostrovsky Jan Beulich Juergen Gross Julien Grall Razvan Cojocaru Vitaly Kuznetsov Wei Liu jobs: build-amd64-xsm pass build-arm64-xsm
Re: [Xen-devel] [PATCH] xen/balloon: add runtime control for scrubbing ballooned out pages
On Thu, Sep 06, 2018 at 04:14:50PM -0400, Boris Ostrovsky wrote: > You can also use cmdline_find_option() in the balloon driver. This looks to be x86 only (?!), which doesn't sound right in drivers/xen/. I'll use core_param() for now... -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? signature.asc Description: PGP signature ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 127347: tolerable all pass - PUSHED
flight 127347 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/127347/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen a6c501603ca886458b055477532abadfbc9d3606 baseline version: xen c1bace00de265bf34bdf6119d99888bbb57a4ef4 Last test of basis 127341 2018-09-06 15:00:47 Z0 days Testing same since 127347 2018-09-06 18:00:43 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Jan Beulich Paul Durrant 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-i386 pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git c1bace00de..a6c501603c a6c501603ca886458b055477532abadfbc9d3606 -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen: avoid crash in disable_hotplug_cpu
Am Thu, 6 Sep 2018 14:45:57 -0400 schrieb Boris Ostrovsky : > On 09/06/2018 02:37 AM, Olaf Hering wrote: > > The command 'xl vcpu-set 0 0', issued in dom0, will crash dom0: > > This happens because handle_vcpu_hotplug_event is called twice. In the > > first iteration cpu_present is still true, in the second iteration > > cpu_present is false which causes get_cpu_device to return NULL. > > In case of cpu#0, cpu_online is apparently always true. > I think we should check both this and num_online_cpus() != 0. This can not possibly help. cpu#0 is the first one that goes offline. IF cpu0_hotpluggable is broken, then only "if (!cpu) return;" can help. Olaf pgpq2mLYOekTt.pgp Description: Digitale Signatur von OpenPGP ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/balloon: add runtime control for scrubbing ballooned out pages
On Thu, Sep 06, 2018 at 04:14:50PM -0400, Boris Ostrovsky wrote: > On 09/06/2018 11:33 AM, Marek Marczykowski-Górecki wrote: > > Scrubbing pages on initial balloon down can take some time, especially > > in nested virtualization case (nested EPT is slow). When HVM/PVH guest is > > started with memory= significantly lower than maxmem=, all the extra > > pages will be scrubbed before returning to Xen. But since most of them > > weren't used at all at that point, Xen needs to populate them first > > (from populate-on-demand pool). In nested virt case (Xen inside KVM) > > this slows down the guest boot by 15-30s with just 1.5GB needed to be > > returned to Xen. > > > > Add runtime parameter to enable/disable it, to allow initially disabling > > scrubbing, then enable it back during boot (for example in initramfs). > > Such usage relies on assumption that a) most pages ballooned out during > > initial boot weren't used at all, and b) even if they were, very few > > secrets are in the guest at that time (before any serious userspace > > kicks in). > > > > Default behaviour is unchanged. > > > > Signed-off-by: Marek Marczykowski-Górecki > > > > --- > > Is module_param() a good thing for this? Other xen-balloon parameters are > > in /sys/devices/system/xen_memory, so maybe it would make sense to put > > this one there too? But then, cmdline parameter would need to be added > > separately and comment about core_param() suggests it shouldn't be used > > if not absolutely necessary (is it?). > > > You can also use cmdline_find_option() in the balloon driver. > > I would prefer that all tunables for the balloon driver live in the same > place. (Note that in that case > Documentation/ABI/stable/sysfs-devices-system-xen_memory will need to be > updated). Ok, will move that. This will also mean that it will affect only balloon driver, not other users of mem-reservation.c (grant allocator). But given the reason for this tunable, I think that's fine. -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? signature.asc Description: PGP signature ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/balloon: add runtime control for scrubbing ballooned out pages
On 09/06/2018 11:33 AM, Marek Marczykowski-Górecki wrote: > Scrubbing pages on initial balloon down can take some time, especially > in nested virtualization case (nested EPT is slow). When HVM/PVH guest is > started with memory= significantly lower than maxmem=, all the extra > pages will be scrubbed before returning to Xen. But since most of them > weren't used at all at that point, Xen needs to populate them first > (from populate-on-demand pool). In nested virt case (Xen inside KVM) > this slows down the guest boot by 15-30s with just 1.5GB needed to be > returned to Xen. > > Add runtime parameter to enable/disable it, to allow initially disabling > scrubbing, then enable it back during boot (for example in initramfs). > Such usage relies on assumption that a) most pages ballooned out during > initial boot weren't used at all, and b) even if they were, very few > secrets are in the guest at that time (before any serious userspace > kicks in). > > Default behaviour is unchanged. > > Signed-off-by: Marek Marczykowski-Górecki > > --- > Is module_param() a good thing for this? Other xen-balloon parameters are > in /sys/devices/system/xen_memory, so maybe it would make sense to put > this one there too? But then, cmdline parameter would need to be added > separately and comment about core_param() suggests it shouldn't be used > if not absolutely necessary (is it?). You can also use cmdline_find_option() in the balloon driver. I would prefer that all tunables for the balloon driver live in the same place. (Note that in that case Documentation/ABI/stable/sysfs-devices-system-xen_memory will need to be updated). -boris ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/3] xen/vcpu: Rework sanity checks in vcpu_create()
On Thu, Sep 6, 2018 at 3:28 PM Andrew Cooper wrote: > > Poisoning idle_vcpu[0] with the sanity debug value isn't actually a clever > idea, because it passes a NULL pointer check but isn't a usable vcpu. It is > also the reason for the (!is_idle_domain(d) || vcpu_id) part of the existing > sanity BUG_ON(). > > Now that d->max_vcpus is appropriately set up before vcpu_create() is called, > we can properly range check the requested vcpu_id. Drop the BUG_ON() and > replace it with code which is runtime safe but non-fatal. > > While v0 must be the first allocated vcpu for for_each_vcpu() to work, it > isn't a requirement for the threading the vcpu into the linked list, so update > the threading code to be more generic, and add a comment explaining why we > need to search for prev_id. > > Signed-off-by: Andrew Cooper > --- > CC: Jan Beulich > CC: Wei Liu > CC: Roger Pau Monné > CC: Stefano Stabellini > CC: Julien Grall > --- > xen/arch/arm/setup.c | 1 - > xen/arch/x86/setup.c | 1 - > xen/common/domain.c | 32 > 3 files changed, 28 insertions(+), 6 deletions(-) > > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > index 01b..d06ac40 100644 > --- a/xen/arch/arm/setup.c > +++ b/xen/arch/arm/setup.c > @@ -704,7 +704,6 @@ void __init start_xen(unsigned long boot_phys_offset, > set_processor_id(0); /* needed early, for smp_processor_id() */ > > set_current((struct vcpu *)0xf000); /* debug sanity */ > -idle_vcpu[0] = current; > > setup_virtual_regions(NULL, NULL); > /* Initialize traps early allow us to get backtrace when an error > occurred */ > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c > index a2f22a1..5e1e8ae 100644 > --- a/xen/arch/x86/setup.c > +++ b/xen/arch/x86/setup.c > @@ -691,7 +691,6 @@ void __init noreturn __start_xen(unsigned long mbi_p) > > set_processor_id(0); > set_current(INVALID_VCPU); /* debug sanity. */ > -idle_vcpu[0] = current; xen/arch/x86/tboot.c:tboot_shutdown() has: if ( idle_vcpu[0] != INVALID_VCPU ) write_ptbase(idle_vcpu[0]); With your change, this should be update to check for non-NULL. Regards, Jason ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/manage: don't complain about an empty value in control/sysrq node
On 09/06/2018 07:26 AM, Vitaly Kuznetsov wrote: > When guest receives a sysrq request from the host it acknowledges it by > writing '\0' to control/sysrq xenstore node. This, however, make xenstore > watch fire again but xenbus_scanf() fails to parse empty value with "%c" > format string: > > sysrq: SysRq : Emergency Sync > Emergency Sync complete > xen:manage: Error -34 reading sysrq code in control/sysrq > > Ignore -ERANGE the same way we already ignore -ENOENT, empty value in > control/sysrq is totally legal. > > Signed-off-by: Vitaly Kuznetsov > Applied to for-linus-19b. -boris ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 0/3] xen: Improvements to the vcpu create/destroy paths
This is the start of the work to make the vcpu_destroy() code idempotent, so vcpu construction can be moved into domain_create() and successfully unwound on error. Andrew Cooper (3): xen/vcpu: Rename the common interfaces for consistency xen/vcpu: Introduce vcpu_destroy() xen/vcpu: Rework sanity checks in vcpu_create() xen/arch/arm/domain.c | 6 ++-- xen/arch/arm/domain_build.c | 4 +-- xen/arch/arm/setup.c| 1 - xen/arch/x86/dom0_build.c | 2 +- xen/arch/x86/domain.c | 4 +-- xen/arch/x86/setup.c| 1 - xen/common/domain.c | 80 ++--- xen/common/domctl.c | 2 +- xen/common/schedule.c | 4 +-- xen/include/xen/domain.h| 10 +++--- 10 files changed, 69 insertions(+), 45 deletions(-) -- 2.1.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 1/3] xen/vcpu: Rename the common interfaces for consistency
The vcpu functions are far less consistent than the domain side of things, and
in particular, has vcpu_destroy() for architecture specific functionality.
Perform the following renames:
* alloc_vcpu => vcpu_create
* vcpu_initialise => arch_vcpu_create
* vcpu_destroy=> arch_vcpu_destroy
which makes the vcpu hierarchy consistent with the domain hierarchy.
No functional change.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
CC: Stefano Stabellini
CC: Julien Grall
---
xen/arch/arm/domain.c | 6 +++---
xen/arch/arm/domain_build.c | 4 ++--
xen/arch/x86/dom0_build.c | 2 +-
xen/arch/x86/domain.c | 4 ++--
xen/common/domain.c | 6 +++---
xen/common/domctl.c | 2 +-
xen/common/schedule.c | 4 ++--
xen/include/xen/domain.h| 10 +-
8 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 4baecc2..feebbf5 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -538,7 +538,7 @@ void free_vcpu_struct(struct vcpu *v)
free_xenheap_pages(v, get_order_from_bytes(sizeof(*v)));
}
-int vcpu_initialise(struct vcpu *v)
+int arch_vcpu_create(struct vcpu *v)
{
int rc = 0;
@@ -583,11 +583,11 @@ int vcpu_initialise(struct vcpu *v)
return rc;
fail:
-vcpu_destroy(v);
+arch_vcpu_destroy(v);
return rc;
}
-void vcpu_destroy(struct vcpu *v)
+void arch_vcpu_destroy(struct vcpu *v)
{
vcpu_timer_destroy(v);
vcpu_vgic_free(v);
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 2a383c8..5057ad8 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -74,7 +74,7 @@ unsigned int __init dom0_max_vcpus(void)
struct vcpu *__init alloc_dom0_vcpu0(struct domain *dom0)
{
-return alloc_vcpu(dom0, 0, 0);
+return vcpu_create(dom0, 0, 0);
}
static unsigned int __init get_11_allocation_size(paddr_t size)
@@ -2232,7 +2232,7 @@ int __init construct_dom0(struct domain *d)
for ( i = 1, cpu = 0; i < d->max_vcpus; i++ )
{
cpu = cpumask_cycle(cpu, &cpu_online_map);
-if ( alloc_vcpu(d, i, cpu) == NULL )
+if ( vcpu_create(d, i, cpu) == NULL )
{
printk("Failed to allocate dom0 vcpu %d on pcpu %d\n", i, cpu);
break;
diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index 423fdec..86eb7db 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -134,7 +134,7 @@ struct vcpu *__init dom0_setup_vcpu(struct domain *d,
unsigned int prev_cpu)
{
unsigned int cpu = cpumask_cycle(prev_cpu, &dom0_cpus);
-struct vcpu *v = alloc_vcpu(d, vcpu_id, cpu);
+struct vcpu *v = vcpu_create(d, vcpu_id, cpu);
if ( v )
{
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 313ebb3..d67a047 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -322,7 +322,7 @@ void free_vcpu_struct(struct vcpu *v)
free_xenheap_page(v);
}
-int vcpu_initialise(struct vcpu *v)
+int arch_vcpu_create(struct vcpu *v)
{
struct domain *d = v->domain;
int rc;
@@ -382,7 +382,7 @@ int vcpu_initialise(struct vcpu *v)
return rc;
}
-void vcpu_destroy(struct vcpu *v)
+void arch_vcpu_destroy(struct vcpu *v)
{
xfree(v->arch.vm_event);
v->arch.vm_event = NULL;
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 78c450e..14c8d00 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -123,7 +123,7 @@ static void vcpu_info_reset(struct vcpu *v)
v->vcpu_info_mfn = INVALID_MFN;
}
-struct vcpu *alloc_vcpu(
+struct vcpu *vcpu_create(
struct domain *d, unsigned int vcpu_id, unsigned int cpu_id)
{
struct vcpu *v;
@@ -165,7 +165,7 @@ struct vcpu *alloc_vcpu(
if ( sched_init_vcpu(v, cpu_id) != 0 )
goto fail_wq;
-if ( vcpu_initialise(v) != 0 )
+if ( arch_vcpu_create(v) != 0 )
{
sched_destroy_vcpu(v);
fail_wq:
@@ -870,7 +870,7 @@ static void complete_domain_destroy(struct rcu_head *head)
if ( (v = d->vcpu[i]) == NULL )
continue;
tasklet_kill(&v->continue_hypercall_tasklet);
-vcpu_destroy(v);
+arch_vcpu_destroy(v);
sched_destroy_vcpu(v);
destroy_waitqueue_vcpu(v);
}
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index ed047b7..2facbdb 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -571,7 +571,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t)
u_domctl)
cpumask_any(online) :
cpumask_cycle(d->vcpu[i-1]->processor, online);
-if ( alloc_vcpu(d, i, cpu) == NULL )
+if ( vcpu_create(d, i, cpu) == NULL )
goto maxvcpu_out;
}
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index 05281d6..1ef5be9 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -
[Xen-devel] [PATCH 3/3] xen/vcpu: Rework sanity checks in vcpu_create()
Poisoning idle_vcpu[0] with the sanity debug value isn't actually a clever
idea, because it passes a NULL pointer check but isn't a usable vcpu. It is
also the reason for the (!is_idle_domain(d) || vcpu_id) part of the existing
sanity BUG_ON().
Now that d->max_vcpus is appropriately set up before vcpu_create() is called,
we can properly range check the requested vcpu_id. Drop the BUG_ON() and
replace it with code which is runtime safe but non-fatal.
While v0 must be the first allocated vcpu for for_each_vcpu() to work, it
isn't a requirement for the threading the vcpu into the linked list, so update
the threading code to be more generic, and add a comment explaining why we
need to search for prev_id.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
CC: Stefano Stabellini
CC: Julien Grall
---
xen/arch/arm/setup.c | 1 -
xen/arch/x86/setup.c | 1 -
xen/common/domain.c | 32
3 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 01b..d06ac40 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -704,7 +704,6 @@ void __init start_xen(unsigned long boot_phys_offset,
set_processor_id(0); /* needed early, for smp_processor_id() */
set_current((struct vcpu *)0xf000); /* debug sanity */
-idle_vcpu[0] = current;
setup_virtual_regions(NULL, NULL);
/* Initialize traps early allow us to get backtrace when an error occurred
*/
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index a2f22a1..5e1e8ae 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -691,7 +691,6 @@ void __init noreturn __start_xen(unsigned long mbi_p)
set_processor_id(0);
set_current(INVALID_VCPU); /* debug sanity. */
-idle_vcpu[0] = current;
init_shadow_spec_ctrl_state();
percpu_init_areas();
diff --git a/xen/common/domain.c b/xen/common/domain.c
index a9df589..d23b54a 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -138,7 +138,19 @@ struct vcpu *vcpu_create(
{
struct vcpu *v;
-BUG_ON((!is_idle_domain(d) || vcpu_id) && d->vcpu[vcpu_id]);
+/*
+ * Sanity check some input expectations:
+ * - d->max_vcpus and d->vcpu[] should be set up
+ * - vcpu_id should be bounded by d->max_vcpus
+ * - v0 must be the first-allocated vcpu
+ * - No previous vcpu with this id should be allocated
+ */
+if ( !d->max_vcpus || !d->vcpu || vcpu_id >= d->max_vcpus ||
+ (vcpu_id > 0 && !d->vcpu[0]) || d->vcpu[vcpu_id] )
+{
+ASSERT_UNREACHABLE();
+return NULL;
+}
if ( (v = alloc_vcpu_struct()) == NULL )
return NULL;
@@ -178,15 +190,27 @@ struct vcpu *vcpu_create(
if ( arch_vcpu_create(v) != 0 )
goto fail_sched;
+/* Insert the vcpu into the domain's vcpu list. */
d->vcpu[vcpu_id] = v;
if ( vcpu_id != 0 )
{
int prev_id = v->vcpu_id - 1;
+
+/*
+ * Look for the previously allocated vcpu, and splice into the
+ * next_in_list single linked list.
+ *
+ * All domains other than IDLE have tightly packed vcpu_id's. IDLE
+ * vcpu_id's are derived from hardware CPU id's and can be sparse.
+ */
while ( (prev_id >= 0) && (d->vcpu[prev_id] == NULL) )
prev_id--;
-BUG_ON(prev_id < 0);
-v->next_in_list = d->vcpu[prev_id]->next_in_list;
-d->vcpu[prev_id]->next_in_list = v;
+
+if ( prev_id >= 0 )
+{
+v->next_in_list = d->vcpu[prev_id]->next_in_list;
+d->vcpu[prev_id]->next_in_list = v;
+}
}
/* Must be called after making new vcpu visible to for_each_vcpu(). */
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 2/3] xen/vcpu: Introduce vcpu_destroy()
Like _domain_destroy(), this will eventually idempotently free all parts of a
struct vcpu.
While breaking apart the failure path of vcpu_create(), rework the codeflow to
be in a line at the end of the function for clarity.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
CC: Stefano Stabellini
CC: Julien Grall
---
xen/common/domain.c | 42 ++
1 file changed, 22 insertions(+), 20 deletions(-)
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 14c8d00..a9df589 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -123,6 +123,16 @@ static void vcpu_info_reset(struct vcpu *v)
v->vcpu_info_mfn = INVALID_MFN;
}
+static void vcpu_destroy(struct vcpu *v)
+{
+free_cpumask_var(v->cpu_hard_affinity);
+free_cpumask_var(v->cpu_hard_affinity_tmp);
+free_cpumask_var(v->cpu_hard_affinity_saved);
+free_cpumask_var(v->cpu_soft_affinity);
+
+free_vcpu_struct(v);
+}
+
struct vcpu *vcpu_create(
struct domain *d, unsigned int vcpu_id, unsigned int cpu_id)
{
@@ -147,7 +157,7 @@ struct vcpu *vcpu_create(
!zalloc_cpumask_var(&v->cpu_hard_affinity_tmp) ||
!zalloc_cpumask_var(&v->cpu_hard_affinity_saved) ||
!zalloc_cpumask_var(&v->cpu_soft_affinity) )
-goto fail_free;
+goto fail;
if ( is_idle_domain(d) )
{
@@ -166,18 +176,7 @@ struct vcpu *vcpu_create(
goto fail_wq;
if ( arch_vcpu_create(v) != 0 )
-{
-sched_destroy_vcpu(v);
- fail_wq:
-destroy_waitqueue_vcpu(v);
- fail_free:
-free_cpumask_var(v->cpu_hard_affinity);
-free_cpumask_var(v->cpu_hard_affinity_tmp);
-free_cpumask_var(v->cpu_hard_affinity_saved);
-free_cpumask_var(v->cpu_soft_affinity);
-free_vcpu_struct(v);
-return NULL;
-}
+goto fail_sched;
d->vcpu[vcpu_id] = v;
if ( vcpu_id != 0 )
@@ -197,6 +196,15 @@ struct vcpu *vcpu_create(
domain_update_node_affinity(d);
return v;
+
+ fail_sched:
+sched_destroy_vcpu(v);
+ fail_wq:
+destroy_waitqueue_vcpu(v);
+ fail:
+vcpu_destroy(v);
+
+return NULL;
}
static int late_hwdom_init(struct domain *d)
@@ -898,13 +906,7 @@ static void complete_domain_destroy(struct rcu_head *head)
for ( i = d->max_vcpus - 1; i >= 0; i-- )
if ( (v = d->vcpu[i]) != NULL )
-{
-free_cpumask_var(v->cpu_hard_affinity);
-free_cpumask_var(v->cpu_hard_affinity_tmp);
-free_cpumask_var(v->cpu_hard_affinity_saved);
-free_cpumask_var(v->cpu_soft_affinity);
-free_vcpu_struct(v);
-}
+vcpu_destroy(v);
if ( d->target != NULL )
put_domain(d->target);
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen: avoid crash in disable_hotplug_cpu
On 09/06/2018 02:37 AM, Olaf Hering wrote:
> The command 'xl vcpu-set 0 0', issued in dom0, will crash dom0:
>
> BUG: unable to handle kernel NULL pointer dereference at 02d8
> PGD 0 P4D 0
> Oops: [#1] PREEMPT SMP NOPTI
> CPU: 7 PID: 65 Comm: xenwatch Not tainted 4.19.0-rc2-1.ga9462db-default #1
> openSUSE Tumbleweed (unreleased)
> Hardware name: Intel Corporation S5520UR/S5520UR, BIOS
> S5500.86B.01.00.0050.050620101605 05/06/2010
> RIP: e030:device_offline+0x9/0xb0
> Code: 77 24 00 e9 ce fe ff ff 48 8b 13 e9 68 ff ff ff 48 8b 13 e9 29 ff ff ff
> 48 8b 13 e9 ea fe ff ff 90 66 66 66 66 90 41 54 55 53 87 d8 02 00 00 01
> 0f 85 88 00 00 00 48 c7 c2 20 09 60 81 31 f6
> RSP: e02b:c90040f27e80 EFLAGS: 00010203
> RAX: RBX: RCX:
> RDX: 8801f380 RSI: c90040f27e70 RDI:
> RBP: R08: 820e47b3 R09:
> R10: 7ff0 R11: R12: 822e6d30
> R13: dead0200 R14: dead0100 R15: 8158b4e0
> FS: 7ffa595158c0() GS:8801f39c() knlGS:
> CS: e033 DS: ES: CR0: 80050033
> CR2: 02d8 CR3: 0001d9602000 CR4: 2660
> Call Trace:
> handle_vcpu_hotplug_event+0xb5/0xc0
> xenwatch_thread+0x80/0x140
> ? wait_woken+0x80/0x80
> kthread+0x112/0x130
> ? kthread_create_worker_on_cpu+0x40/0x40
> ret_from_fork+0x3a/0x50
>
> This happens because handle_vcpu_hotplug_event is called twice. In the
> first iteration cpu_present is still true, in the second iteration
> cpu_present is false which causes get_cpu_device to return NULL.
> In case of cpu#0, cpu_online is apparently always true.
>
> Fix this crash by checking if the cpu can be hotplugged, which is false
> for a cpu that was just removed.
>
> Signed-off-by: Olaf Hering
> ---
> drivers/xen/cpu_hotplug.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/xen/cpu_hotplug.c b/drivers/xen/cpu_hotplug.c
> index d4265c8ebb22..bf1e41ed9d41 100644
> --- a/drivers/xen/cpu_hotplug.c
> +++ b/drivers/xen/cpu_hotplug.c
> @@ -19,6 +19,8 @@ static void enable_hotplug_cpu(int cpu)
>
> static void disable_hotplug_cpu(int cpu)
> {
> + if (!cpu_is_hotpluggable(cpu))
I think we should check both this and num_online_cpus() != 0.
Even though I don't believe cpu0_hotpluggable currently works, at some
point it might.
-boris
> + return;
> if (cpu_online(cpu)) {
> lock_device_hotplug();
> device_offline(get_cpu_device(cpu));
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] tools: specifically enable VirtFS in Linux QEMU builds
On Thu, 6 Sep 2018, Paul Durrant wrote:
> 9pfs support has been a documented feature since Xen 4.9, but QEMU will
> not be built with backend support unless libcap and libattr dev packages
> are installed.
>
> This patch modifies the README to call out those packages as pre-requisites
> for Linux builds and specifically enables VirtFS in the configure line
> for QEMU so that an error message is displayed if they are missing.
>
> Signed-off-by: Paul Durrant
Thank you, Paul! Do we need to do anything for the configure stuff
(AC_CHECK_LIB in tools/configure.ac)?
> ---
> Cc: Andrew Cooper
> Cc: George Dunlap
> Cc: Ian Jackson
> Cc: Jan Beulich
> Cc: Julien Grall
> Cc: Konrad Rzeszutek Wilk
> Cc: Stefano Stabellini
> Cc: Tim Deegan
> Cc: Wei Liu
> ---
> README | 2 ++
> tools/Makefile | 11 +++
> 2 files changed, 13 insertions(+)
>
> diff --git a/README b/README
> index 4b95b21c7b..1a4e4b2c1b 100644
> --- a/README
> +++ b/README
> @@ -56,6 +56,8 @@ provided by your OS distributor:
>greater.
> * Development install of GLib v2.0 (e.g. libglib2.0-dev)
> * Development install of Pixman (e.g. libpixman-1-dev)
> +* Development install of libcap (e.g. libcap-dev) [Linux only]
> +* Development install of libattr (e.g. libattr1-dev) [Linux only]
> * pkg-config
> * bridge-utils package (/sbin/brctl)
> * iproute package (/sbin/ip)
> diff --git a/tools/Makefile b/tools/Makefile
> index 67977ad850..e74efb8a6e 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -216,6 +216,11 @@ else
> QEMU_XEN_ENABLE_DEBUG :=
> endif
>
> +#
> +# 9pfs support is a documented feature but it depends on a QEMU with
> +# VirtFS enabled. However VirtFS is a Linux-only option so only enable
> +# it for Linux builds.
> +#
> subdir-all-qemu-xen-dir: qemu-xen-dir-find
> unset MAKELEVEL; \
> if test -d $(QEMU_UPSTREAM_LOC) ; then \
> @@ -232,10 +237,16 @@ subdir-all-qemu-xen-dir: qemu-xen-dir-find
> else \
> enable_trace_backend='' ; \
> fi ; \
> + if [ "$(CONFIG_Linux)" = "y" ]; then \
> + enable_virtfs='--enable-virtfs' ; \
> + else \
> + enable_virtfs='' ; \
> + fi ; \
>
> PKG_CONFIG_PATH=$(XEN_ROOT)/tools/pkg-config$${PKG_CONFIG_PATH:+:$${PKG_CONFIG_PATH}}
> \
> $$source/configure --enable-xen --target-list=i386-softmmu \
> $(QEMU_XEN_ENABLE_DEBUG) \
> $$enable_trace_backend \
> + $$enable_virtfs \
> --prefix=$(LIBEXEC) \
> --libdir=$(LIBEXEC_LIB) \
> --includedir=$(LIBEXEC_INC) \
> --
> 2.11.0
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 127341: tolerable all pass - PUSHED
flight 127341 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/127341/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen c1bace00de265bf34bdf6119d99888bbb57a4ef4 baseline version: xen 2c0b1824b1cb33a2610f3f55299247f9e0464466 Last test of basis 127312 2018-09-05 14:00:41 Z1 days Testing same since 127341 2018-09-06 15:00:47 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Jan Beulich Olaf Hering Paul Durrant 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-i386 pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 2c0b1824b1..c1bace00de c1bace00de265bf34bdf6119d99888bbb57a4ef4 -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [ovmf test] 127335: all pass - PUSHED
flight 127335 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/127335/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 22cf747fcf75dbfe51e5524ce1f9cf17b19914cd baseline version: ovmf c3d5d800d775095e0bbd0ac8862df234cdd4332d Last test of basis 127318 2018-09-05 18:41:01 Z0 days Testing same since 127335 2018-09-06 09:51:21 Z0 days1 attempts People who touched revisions under test: Chen A Chen Dandan Bi Eric Dong Ruiyu Ni shenglei Star Zeng Zhiju.Fan zhijufan 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 c3d5d800d7..22cf747fcf 22cf747fcf75dbfe51e5524ce1f9cf17b19914cd -> xen-tested-master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] automation: specify -j$(nproc) in build script
On Thu, Sep 06, 2018 at 03:55:59PM +0100, Wei Liu wrote:
> Signed-off-by: Wei Liu
> ---
> automation/scripts/build | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/automation/scripts/build b/automation/scripts/build
> index 054226bd73..c463b060d4 100755
> --- a/automation/scripts/build
> +++ b/automation/scripts/build
> @@ -30,4 +30,4 @@ fi
>
> ./configure "${cfgargs[@]}"
>
> -make dist
> +make -j$(nproc) dist
> --
> 2.11.0
>
Acked-by: Doug Goldstein
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/5] x86/hvm: Make HVM_PARAM_{STORE, CONSOLE}_EVTCHN read-only to the guest
On 06/09/18 16:29, Andrew Cooper wrote:
On 06/09/18 10:16, Paul Durrant wrote:
-Original Message-
From: Andrew Cooper [mailto:andrew.coop...@citrix.com]
Sent: 05 September 2018 19:12
To: Xen-devel
Cc: Andrew Cooper ; Jan Beulich
; Wei Liu ; Roger Pau Monne
; Paul Durrant ; Stefano
Stabellini ; Julien Grall
Subject: [PATCH 3/5] x86/hvm: Make
HVM_PARAM_{STORE,CONSOLE}_EVTCHN read-only to the guest
These values are set by the toolstack for each create/restore operation, and
bound by xen{store,console}d before the the guest starts running.
A guest has no reason to modify them at all, and the matching *_PFN
parameters
are already read-only. Adjust the *_EVTCHN permissions to be consistent.
Unfortunately this patch will break the Windows PV driver function here:
http://xenbits.xen.org/gitweb/?p=pvdrivers/win/xenbus.git;a=blob;f=src/xenbus/evtchn.c;hb=HEAD#l1037
Unfortunately the values really do change across a reset. It would be possible
to use volatile (disappear on reboot) registry keys to store the updated values
instead but I don't really see any harm in allowing the guest to update the
values to be correct, unless we want to change Xen to do the job so the guest
doesn't have to go through this dance.
:( Everything is terrible.
This is a general problem, not x86 specific, so I'll drop this patch and
make a similar adjustment to the ARM one.
I am a bit confused. I would have thought this was updated by the
toolstack at reset. So why would the guest update them?
Cheers,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [libvirt test] 127329: regressions - FAIL
flight 127329 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/127329/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-i386-libvirt6 libvirt-buildfail REGR. vs. 123814 build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 123814 build-arm64-libvirt 6 libvirt-buildfail REGR. vs. 123814 build-armhf-libvirt 6 libvirt-buildfail REGR. vs. 123814 Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a version targeted for testing: libvirt 4b7b0c24535af6c410886dc941a72e0622e9ea47 baseline version: libvirt 076a2b409667dd9f716a2a2085e1ffea9d58fe8b Last test of basis 123814 2018-06-05 04:19:23 Z 93 days Failing since123840 2018-06-06 04:19:28 Z 92 days 74 attempts Testing same since 127329 2018-09-06 04:18:56 Z0 days1 attempts People who touched revisions under test: Ales Musil Andrea Bolognani Anya Harter Bing Niu Bjoern Walk Bobo Du Boris Fiuczynski Brijesh Singh Changkuo Shi Chen Hanxiao Christian Ehrhardt Clementine Hayat Cole Robinson Dan Kenigsberg Daniel Nicoletti Daniel P. Berrangé Daniel Veillard Eric Blake Erik Skultety Fabiano Fidêncio Farhan Ali Filip Alac Han Han intrigeri intrigeri Jamie Strandboge Jie Wang Jim Fehlig Jiri Denemark John Ferlan Julio Faracco Ján Tomko Kashyap Chamarthy Katerina Koukiou Laine Stump Laszlo Ersek Lubomir Rintel Luyao Huang Marc Hartmayer Marc Hartmayer Marcos Paulo de Souza Marek Marczykowski-Górecki Martin Kletzander Matthias Bolte Michal Privoznik Michal Prívozník Nikolay Shirokovskiy Pavel Hrdina Peter Krempa Pino Toscano Radostin Stoyanov Ramy Elkest ramyelkest Richard W.M. Jones Roman Bogorodskiy Roman Bolshakov Shi Lei Shichangkuo Shivaprasad G Bhat Simon Kobyda Stefan Bader Stefan Berger Sukrit Bhatnagar Tomáš Golembiovský Vitaly Kuznetsov w00251574 Weilun Zhu xinhua.Cao jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt fail build-arm64-libvirt fail build-armhf-libvirt fail build-i386-libvirt fail build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm blocked test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmblocked test-amd64-amd64-libvirt-xsm blocked test-arm64-arm64-libvirt-xsm blocked test-amd64-i386-libvirt-xsm blocked test-amd64-amd64-libvirt blocked test-arm64-arm64-libvirt blocked test-armhf-armhf-libvirt blocked test-amd64-i386-libvirt
Re: [Xen-devel] [PATCH 5/6] xen/bitmap: Drop all bitmap_scn{, list}printf() infrastructure
On Thu, Sep 06, 2018 at 01:08:15PM +0100, Andrew Cooper wrote: > All callers have been convered to using %*pb[l]. In the unlikely case that > future code wants to retain this functionaly, it can be replicated in a more > convenient fashon with snprintf(). > > Signed-off-by: Andrew Cooper Acked-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 4/6] xen/x86: Use %*pb[l] instead of cpumask_scn{, list}printf()
On Thu, Sep 06, 2018 at 01:08:14PM +0100, Andrew Cooper wrote: > This removes all use of keyhandler_scratch as a bounce-buffer for the rendered > string. > > No functional change. > > Signed-off-by: Andrew Cooper Reviewed-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/6] xen/vsprintf: Introduce %*pb[l] for printing bitmaps
On Thu, Sep 06, 2018 at 01:08:11PM +0100, Andrew Cooper wrote:
> The format identifier is consistent with Linux. The code is adapted from
> bitmap_scn{,list}printf() but cleaned up.
>
> This change allows all callers to avoid needing a secondary buffer to render a
> cpumask/nodemask into.
>
> Signed-off-by: Andrew Cooper
Acked-by: Wei Liu
I only skim-read this patch, since this is mostly moved / copied code.
Wei.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC 6/6] xen/keyhandler: Drop keyhandler_scratch
On Thu, Sep 06, 2018 at 01:08:16PM +0100, Andrew Cooper wrote: > With almost all users of keyhandler_scratch gone, clean up the 3 remaining > users and drop the buffer. > > Signed-off-by: Andrew Cooper > --- > CC: Jan Beulich > CC: Wei Liu > CC: Roger Pau Monné > > The NUMA and periodic timer adjustments are definitely improvements. However, > this is RFC because of the EFI change, which might perhaps not wan to be done > this way. Suggestions welcome. Why would the EFI change be a problem? Using keyhandler_scratch as a generic buffer in EFI code seems wrong to me anyway. Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/6] xen/common: Use %*pb[l] instead of {cpu, node}mask_scn{, list}printf()
On Thu, Sep 06, 2018 at 01:08:13PM +0100, Andrew Cooper wrote:
> This removes all use of keyhandler_scratch as a bounce-buffer for the rendered
> string. In some cases, collapse combine adjacent printk()'s which are writing
> parts of the same line.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper
Reviewed-by: Wei Liu
> ---
> CC: Jan Beulich
> CC: Wei Liu
> CC: Roger Pau Monné
> CC: Stefano Stabellini
> CC: Julien Grall
> CC: Juergen Gross
> ---
> xen/common/cpupool.c | 12 +++-
> xen/common/event_channel.c | 6 ++
> xen/common/keyhandler.c| 35 +--
> 3 files changed, 14 insertions(+), 39 deletions(-)
>
> diff --git a/xen/common/cpupool.c b/xen/common/cpupool.c
> index 1e8edcb..16ca4c4 100644
> --- a/xen/common/cpupool.c
> +++ b/xen/common/cpupool.c
> @@ -732,12 +732,6 @@ int cpupool_do_sysctl(struct xen_sysctl_cpupool_op *op)
> return ret;
> }
>
> -static void print_cpumap(const char *str, const cpumask_t *map)
> -{
> -cpulist_scnprintf(keyhandler_scratch, sizeof(keyhandler_scratch), map);
> -printk("%s: %s\n", str, keyhandler_scratch);
> -}
> -
> void dump_runq(unsigned char key)
> {
> unsigned longflags;
> @@ -751,17 +745,17 @@ void dump_runq(unsigned char key)
> sched_smt_power_savings? "enabled":"disabled");
> printk("NOW=%"PRI_stime"\n", now);
>
> -print_cpumap("Online Cpus", &cpu_online_map);
> +printk("Online Cpus: %*pbl\n", nr_cpu_ids, &cpu_online_map);
> if ( !cpumask_empty(&cpupool_free_cpus) )
> {
> -print_cpumap("Free Cpus", &cpupool_free_cpus);
> +printk("Free Cpus: %*pbl\n", nr_cpu_ids, &cpupool_free_cpus);
> schedule_dump(NULL);
> }
>
> for_each_cpupool(c)
> {
> printk("Cpupool %d:\n", (*c)->cpupool_id);
> -print_cpumap("Cpus", (*c)->cpu_valid);
> +printk("Cpus: %*pbl\n", nr_cpu_ids, (*c)->cpu_valid);
> schedule_dump(*c);
> }
>
> diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
> index 381f30e..f34d4f0 100644
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -1377,11 +1377,9 @@ static void domain_dump_evtchn_info(struct domain *d)
> unsigned int port;
> int irq;
>
> -bitmap_scnlistprintf(keyhandler_scratch, sizeof(keyhandler_scratch),
> - d->poll_mask, d->max_vcpus);
> printk("Event channel information for domain %d:\n"
> - "Polling vCPUs: {%s}\n"
> - "port [p/m/s]\n", d->domain_id, keyhandler_scratch);
> + "Polling vCPUs: {%*pbl}\n"
> + "port [p/m/s]\n", d->domain_id, d->max_vcpus, d->poll_mask);
>
> spin_lock(&d->event_lock);
>
> diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c
> index 777c8e9..93ae738 100644
> --- a/xen/common/keyhandler.c
> +++ b/xen/common/keyhandler.c
> @@ -250,22 +250,6 @@ static void reboot_machine(unsigned char key, struct
> cpu_user_regs *regs)
> machine_restart(0);
> }
>
> -static void cpuset_print(char *set, int size, const cpumask_t *mask)
> -{
> -*set++ = '{';
> -set += cpulist_scnprintf(set, size-2, mask);
> -*set++ = '}';
> -*set++ = '\0';
> -}
> -
> -static void nodeset_print(char *set, int size, const nodemask_t *mask)
> -{
> -*set++ = '[';
> -set += nodelist_scnprintf(set, size-2, mask);
> -*set++ = ']';
> -*set++ = '\0';
> -}
> -
> static void periodic_timer_print(char *str, int size, uint64_t period)
> {
> if ( period == 0 )
> @@ -298,14 +282,14 @@ static void dump_domains(unsigned char key)
> process_pending_softirqs();
>
> printk("General information for domain %u:\n", d->domain_id);
> -cpuset_print(tmpstr, sizeof(tmpstr), d->dirty_cpumask);
> printk("refcnt=%d dying=%d pause_count=%d\n",
> atomic_read(&d->refcnt), d->is_dying,
> atomic_read(&d->pause_count));
> printk("nr_pages=%d xenheap_pages=%d shared_pages=%u
> paged_pages=%u "
> - "dirty_cpus=%s max_pages=%u\n", d->tot_pages,
> d->xenheap_pages,
> -atomic_read(&d->shr_pages), atomic_read(&d->paged_pages),
> -tmpstr, d->max_pages);
> + "dirty_cpus={%*pbl} max_pages=%u\n",
> + d->tot_pages, d->xenheap_pages, atomic_read(&d->shr_pages),
> + atomic_read(&d->paged_pages), nr_cpu_ids, d->dirty_cpumask,
> + d->max_pages);
> printk("handle=%02x%02x%02x%02x-%02x%02x-%02x%02x-"
> "%02x%02x-%02x%02x%02x%02x%02x%02x vm_assist=%08lx\n",
> d->handle[ 0], d->handle[ 1], d->handle[ 2], d->handle[ 3],
> @@ -324,8 +308,8 @@ static void dump_domains(unsigned char key)
>
> dump_pageframe_info(d);
>
> -nodeset_print(tmpstr, sizeof(tmpstr), &d->node_affinity);
> -printk("NODE affinity for domain %d: %s\n", d->domain_id, tmpstr);
Re: [Xen-devel] [PATCH] xen-blkback: Switch to closed state after releasing the backing device
On Wed, Sep 05, 2018 at 06:28:01PM +0200, Valentin Vidic wrote: > On Wed, Sep 05, 2018 at 01:35:15PM +0200, Valentin Vidic wrote: > > > AFAICT, this will cause the backend to never switch to 'Closed' state > > > until the toolstack sets online to 0, which is not good IMO. > > > > > > If for example a frontend decides to close a device, the backend will > > > stay in state 'Closing' until the toolstack actually removes the disk > > > by setting online to 0. > > > > > > This will prevent resetting blk connections, as blkback will refuse to > > > switch to state XenbusStateInitWait unless it's at XenbusStateClosed > > > (see the XenbusStateInitialising case in frontend_changed), which will > > > never be reached with your patch. > > Would it be possible to call xen_vbd_free before the state change? > > case XenbusStateClosed: > xen_blkif_disconnect(be->blkif); > xen_vbd_free(&be->blkif->vbd); > xenbus_switch_state(dev, XenbusStateClosed); I think that will break reconnection, since xen_vbd_create is only called after hotplug script execution is performed (which happens only once at device attachment), but not when DomU changes frontend state. If you want to perform this xen_vbd_free you will also have to move the xen_vbd_create call AFAICT, to a place that's also called when reconnecting a device. Note that I could be wrong, so it might be worth a shot to try different approaches since the blkback code is quite tangled and I might miss something. Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 10/16] x86/mm: put nested p2m code under CONFIG_HVM
On 09/04/2018 05:15 PM, Wei Liu wrote:
> These functions are only useful for nested hvm, which isn't enabled
> when CONFIG_HVM is false.
>
> Enclose relevant code and fields in CONFIG_HVM. Guard np2m_schedule
> with nestedhvm_enabled.
>
> Signed-off-by: Wei Liu
> ---
> xen/arch/x86/domain.c| 6 --
> xen/arch/x86/mm/p2m.c| 18 ++
> xen/include/asm-x86/domain.h | 2 ++
> xen/include/asm-x86/p2m.h| 2 ++
> 4 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 313ebb3221..7c945a2428 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1691,7 +1691,8 @@ void context_switch(struct vcpu *prev, struct vcpu
> *next)
> {
> _update_runstate_area(prev);
> vpmu_switch_from(prev);
> -np2m_schedule(NP2M_SCHEDLE_OUT);
> +if ( nestedhvm_enabled(prevd) )
> +np2m_schedule(NP2M_SCHEDLE_OUT);
> }
>
> if ( is_hvm_domain(prevd) && !list_empty(&prev->arch.hvm.tm_list) )
> @@ -1758,7 +1759,8 @@ void context_switch(struct vcpu *prev, struct vcpu
> *next)
>
> /* Must be done with interrupts enabled */
> vpmu_switch_to(next);
> -np2m_schedule(NP2M_SCHEDLE_IN);
> +if ( nestedhvm_enabled(nextd) )
> +np2m_schedule(NP2M_SCHEDLE_IN);
There's already a nestedhvm_enabled() check first thing in
np2m_schedule(). How does adding this check help the CONFIG_HVM cause?
And why not #ifdef out this call, as well as the np2m_schedule()
functions entirely?
Everything else looks good.
-George
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] libxl: made vm mac address assignment deterministic
Wei Liu writes ("Re: [PATCH] libxl: made vm mac address assignment
deterministic"):
> On Wed, Sep 05, 2018 at 12:25:55PM +, Joshua Perrett wrote:
> > Uses MD5 on the host mac address, vm name and vif index to generate the
> > last three bytes of the vm mac address (for each vm).
There is no such thing as "the" host mac address. The host might
have several. However, generally there is a specific interface that
will be used for this guest, depending on the vif connection mode. In
bridge mode, for example, there is the mac address of the bridge. I
think you should make sure to use the right interface.
I think you need to add something to the documentation. You should
mention that this approach is only deterministic *on the same host*
(so in setups where the guest might be started on multiple hosts, with
networked storage, it won't work) and only *with the same physical
nic* (so swapping out the physical nic will change all the guests'
addresses).
> > MD5 code is originally from the public domain (written by Colin Plumb in
> > 1993), files found in xen/tools/blktap2/drivers/.
You are duplicating these files within the Xen tree. That's pretty
ugly. Can we not do this a better way ?
> > +static int libxl__get_host_mac(libxl__gc *gc, unsigned char *buf)
> > +{
> > +int rc = ERROR_FAIL;
>
> Having a blank line makes things a bit clearer.
>
> > +#ifdef __linux__
> > +struct ifaddrs *iface_list;
Can we not have #ifdefs here please ? The Linux-specific part should
go in libxl_linux.c.
> > +memcpy(&value, s->sll_addr, 6);
>
> Please use sizeof(s->sll_addr) instead of 6.
Err, are you sure ? Let me quote the proposed code:
> > +if (s->sll_halen == 6) {
> > +uint64_t value = 0;
> > +memcpy(&value, s->sll_addr, 6);
This is very strange. How can it possibly be right to overwrite the
first 6 bytes of value ? That does a different thing on big-endian
and little-endian machines. (
Also why are we not checking sll_hatype ?
> > +if (value > largest) {
> > +memcpy(buf, s->sll_addr, 6);
> > +largest = value;
> > +rc = 0;
>
> Interesting algorithm, but I don't know anything better at the moment.
> :)
There is some reason for choosing the lexically largest address, IIRC.
But the reason should be in a comment in the code. (ISTR reading
something about this in the Debian bridge-interfaces(5) manpage.)
Thanks,
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xsm: fix clang build
On Wed, Sep 05, 2018 at 04:46:06PM +0200, Roger Pau Monne wrote: > ebitmap.c:244:32: error: invalid conversion specifier 'Z' > [-Werror,-Wformat-invalid-specifier] >"match my size %Zd (high bit was %d)\n", mapunit, > ~^ > ebitmap.c:245:16: error: format specifies type 'int' but the argument has > type 'unsigned long' > [-Werror,-Wformat] >sizeof(u64) * 8, e->highbit); >^~~ > ebitmap.c:245:33: error: data argument not used by format string > [-Werror,-Wformat-extra-args] >sizeof(u64) * 8, e->highbit); > > Use %zd instead of %Zd, which is compliant with C99. > > Signed-off-by: Roger Pau Monné Reviewed-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen-blkback: Switch to closed state after releasing the backing device
On Wed, Sep 05, 2018 at 06:27:56PM +0200, Valentin Vidic wrote:
> On Wed, Sep 05, 2018 at 12:36:49PM +0200, Roger Pau Monné wrote:
> > On Wed, Aug 29, 2018 at 08:52:14AM +0200, Valentin Vidic wrote:
> > > Switching to closed state earlier can cause the block-drbd
> > > script to fail with 'Device is held open by someone':
> > >
> > > root: /etc/xen/scripts/block-drbd: remove XENBUS_PATH=backend/vbd/6/51712
> > > kernel: [ .278235] block drbd6: State change failed: Device is held
> > > open by someone
> > > kernel: [ .278304] block drbd6: state = { cs:Connected
> > > ro:Primary/Secondary ds:UpToDate/UpToDate r- }
> > > kernel: [ .278340] block drbd6: wanted = { cs:Connected
> > > ro:Secondary/Secondary ds:UpToDate/UpToDate r- }
> > > root: /etc/xen/scripts/block-drbd: Writing
> > > backend/vbd/6/51712/hotplug-error /etc/xen/scripts/block-drbd failed;
> > > error detected. backend/vbd/6/51712/hotplug-status error to xenstore.
> > > root: /etc/xen/scripts/block-drbd: /etc/xen/scripts/block-drbd failed;
> > > error detected.
> > >
> > > Signed-off-by: Valentin Vidic
> > > Cc: sta...@vger.kernel.org
> > > ---
> > > drivers/block/xen-blkback/xenbus.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/block/xen-blkback/xenbus.c
> > > b/drivers/block/xen-blkback/xenbus.c
> > > index a4bc74e72c39..43bddc996709 100644
> > > --- a/drivers/block/xen-blkback/xenbus.c
> > > +++ b/drivers/block/xen-blkback/xenbus.c
> > > @@ -323,6 +323,7 @@ static void xen_blkif_free(struct xen_blkif *blkif)
> > > {
> > > WARN_ON(xen_blkif_disconnect(blkif));
> > > xen_vbd_free(&blkif->vbd);
> > > + xenbus_switch_state(blkif->be->dev, XenbusStateClosed);
> > > kfree(blkif->be->mode);
> > > kfree(blkif->be);
> > >
> > > @@ -814,7 +815,6 @@ static void frontend_changed(struct xenbus_device
> > > *dev,
> > >
> > > case XenbusStateClosed:
> > > xen_blkif_disconnect(be->blkif);
> > > - xenbus_switch_state(dev, XenbusStateClosed);
> > > if (xenbus_dev_is_online(dev))
> > > break;
> >
> > AFAICT, this will cause the backend to never switch to 'Closed' state
> > until the toolstack sets online to 0, which is not good IMO.
> >
> > If for example a frontend decides to close a device, the backend will
> > stay in state 'Closing' until the toolstack actually removes the disk
> > by setting online to 0.
> >
> > This will prevent resetting blk connections, as blkback will refuse to
> > switch to state XenbusStateInitWait unless it's at XenbusStateClosed
> > (see the XenbusStateInitialising case in frontend_changed), which will
> > never be reached with your patch.
>
> Ok, is it possible to test this somehow?
Yes, you can try booting a PV guest with pvgrub, that will cause
pvgrub to open a blk connection, then close it and afterwards the OS
kernel will start a new connection.
Thanks, Roger.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 07/16] x86/p2m/pod: make it build with !CONFIG_HVM
On 09/06/2018 04:05 PM, Jan Beulich wrote:
On 04.09.18 at 18:15, wrote:
>> --- a/xen/arch/x86/mm/p2m.c
>> +++ b/xen/arch/x86/mm/p2m.c
>> @@ -52,15 +52,20 @@ DEFINE_PERCPU_RWLOCK_GLOBAL(p2m_percpu_rwlock);
>> /* Init the datastructures for later use by the p2m code */
>> static int p2m_initialise(struct domain *d, struct p2m_domain *p2m)
>> {
>> -unsigned int i;
>> int ret = 0;
>> +#ifdef CONFIG_HVM
>> +unsigned int i;
>> +#endif
>>
>> mm_rwlock_init(&p2m->lock);
>> -mm_lock_init(&p2m->pod.lock);
>> INIT_LIST_HEAD(&p2m->np2m_list);
>> INIT_PAGE_LIST_HEAD(&p2m->pages);
>> +
>> +#ifdef CONFIG_HVM
>> +mm_lock_init(&p2m->pod.lock);
>> INIT_PAGE_LIST_HEAD(&p2m->pod.super);
>> INIT_PAGE_LIST_HEAD(&p2m->pod.single);
>> +#endif
>>
>> p2m->domain = d;
>> p2m->default_access = p2m_access_rwx;
>> @@ -69,8 +74,10 @@ static int p2m_initialise(struct domain *d, struct
>> p2m_domain *p2m)
>> p2m->np2m_base = P2M_BASE_EADDR;
>> p2m->np2m_generation = 0;
>>
>> +#ifdef CONFIG_HVM
>> for ( i = 0; i < ARRAY_SIZE(p2m->pod.mrp.list); ++i )
>> p2m->pod.mrp.list[i] = gfn_x(INVALID_GFN);
>> +#endif
>
> I wonder whether all of this wouldn't better go into p2m_pod_init()
> (or alike), to limit/avoid the #ifdef-ary here.
The cover letter for this series did say the initial goal was just to
make it build; and cleaning up could be done later. Adding
p2m_pod_init() would be better long term, I think, but I'm also happy
with leaving if #ifdef's for now and cleaning them up later.
>
>> @@ -2560,7 +2573,10 @@ void audit_p2m(struct domain *d,
>> P2M_PRINTK("p2m audit starts\n");
>>
>> p2m_lock(p2m);
>> +
>> +#ifdef CONFIG_HVM
>> pod_lock(p2m);
>> +#endif
>>
>> if (p2m->audit_p2m)
>> pmbad = p2m->audit_p2m(p2m);
>> @@ -2621,7 +2637,9 @@ void audit_p2m(struct domain *d,
>> }
>> spin_unlock(&d->page_alloc_lock);
>>
>> +#ifdef CONFIG_HVM
>> pod_unlock(p2m);
>> +#endif
>
> Perhaps better make them inline stubs? Otoh - isn't the whole
> function useful for HVM only?
Oh yes -- now that we've gotten rid of full translate guests for PV, I
guess so.
>
>> --- a/xen/common/vm_event.c
>> +++ b/xen/common/vm_event.c
>> @@ -630,7 +630,9 @@ int vm_event_domctl(struct domain *d, struct
>> xen_domctl_vm_event_op *vec,
>> {
>> case XEN_VM_EVENT_ENABLE:
>> {
>> +#ifdef CONFIG_HVM
>> struct p2m_domain *p2m = p2m_get_hostp2m(d);
>> +#endif
>>
>> rc = -EOPNOTSUPP;
>> /* hvm fixme: p2m_is_foreign types need addressing */
>> @@ -647,10 +649,12 @@ int vm_event_domctl(struct domain *d, struct
>> xen_domctl_vm_event_op *vec,
>> if ( unlikely(need_iommu(d)) )
>> break;
>>
>> +#ifdef CONFIG_HVM
>> rc = -EXDEV;
>> /* Disallow paging in a PoD guest */
>> if ( p2m->pod.entry_count )
>> break;
>> +#endif
>
> Perhaps simply ditch the local variable?
That seems like a good idea.
Everything else looks good to me.
-George
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-linus test] 127315: regressions - FAIL
flight 127315 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/127315/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-amd64-qemuu-nested-intel 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-i386-rumprun-i386 12 guest-start fail REGR. vs. 125898 test-amd64-amd64-rumprun-amd64 12 guest-startfail REGR. vs. 125898 test-amd64-i386-xl-shadow12 guest-start fail REGR. vs. 125898 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-amd64-libvirt-xsm 12 guest-start fail REGR. vs. 125898 test-amd64-amd64-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-amd64-libvirt-vhd 10 debian-di-installfail REGR. vs. 125898 test-amd64-amd64-pair21 guest-start/debian fail REGR. vs. 125898 test-amd64-amd64-qemuu-nested-amd 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-amd64-libvirt 12 guest-start fail REGR. vs. 125898 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-amd64-xl-pvshim 12 guest-start fail REGR. vs. 125898 test-amd64-i386-freebsd10-i386 11 guest-startfail REGR. vs. 125898 test-amd64-amd64-xl-shadow 12 guest-start fail REGR. vs. 125898 test-amd64-i386-xl-xsm 12 guest-start fail REGR. vs. 125898 test-amd64-i386-xl 12 guest-start fail REGR. vs. 125898 test-amd64-amd64-xl-credit2 12 guest-start fail REGR. vs. 125898 test-amd64-i386-qemuu-rhel6hvm-intel 10 redhat-install fail REGR. vs. 125898 test-amd64-i386-qemuu-rhel6hvm-amd 10 redhat-install fail REGR. vs. 125898 test-amd64-amd64-libvirt-pair 21 guest-start/debian fail REGR. vs. 125898 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-i386-xl-qemuu-ws16-amd64 10 windows-install fail REGR. vs. 125898 test-amd64-i386-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-i386-xl-qemuu-win7-amd64 10 windows-install fail REGR. vs. 125898 test-amd64-amd64-xl-xsm 12 guest-start fail REGR. vs. 125898 test-amd64-amd64-xl-pvhv2-intel 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-qemuu-win7-amd64 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-multivcpu 7 xen-bootfail REGR. vs. 125898 test-amd64-amd64-pygrub 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-qcow2 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-examine 8 reboot fail REGR. vs. 125898 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-install fail REGR. vs. 125898 test-amd64-amd64-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-i386-xl-qemut-debianhvm-amd64 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl-qemut-win10-i386 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-libvirt 12 guest-start fail REGR. vs. 125898 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-i386-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-i386-libvirt-pair 21 guest-start/debian fail REGR. vs. 125898 test-amd64-amd64-xl 12 guest-start fail REGR. vs. 125898 test-amd64-i386-libvirt-xsm 12 guest-start fail REGR. vs. 125898 test-amd64-i386-qemut-rhel6hvm-intel 10 redhat-install fail REGR. vs. 125898 test-amd64-i386-pair 21 guest-start/debian fail REGR. vs. 125898 test-amd64-i386-freebsd10-amd64 11 guest-start fail REGR. vs. 125898 test-amd64-i386-qemut-rhel6hvm-amd 10 redhat-install fail REGR. vs. 125898 test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail REGR. vs. 125898 test-amd64-amd64-xl-qemut-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-amd64-examine 8 reboot fail REGR. vs. 125898 test-amd64-amd64-amd64-pvgrub 10 debian-di-install fail REGR. vs. 125898 test-amd64-i386-xl-raw 10 debian-di-installfail REGR. vs. 125898 test-amd64-amd64-i386-pvgrub 10 debian-di-installfail REGR. vs. 1258
Re: [Xen-devel] [PATCH] xen/ARM+sched: Don't opencode %pv in printk()'s
On 08/30/2018 01:50 PM, Andrew Cooper wrote: > No functional change. > > Signed-off-by: Andrew Cooper Reviewed-by: George Dunlap ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] libxl: made vm mac address assignment deterministic
On Wed, Sep 05, 2018 at 12:25:55PM +, Joshua Perrett wrote:
> Uses MD5 on the host mac address, vm name and vif index to generate the
> last three bytes of the vm mac address (for each vm).
>
> It uses the vif index to account for multiple vifs.
>
> MD5 code is originally from the public domain (written by Colin Plumb in
> 1993), files found in xen/tools/blktap2/drivers/.
>
> Reported-by: George Dunlap
> Signed-off-by: Joshua Perrett
[...]
> int libxl_mac_to_device_nic(libxl_ctx *ctx, uint32_t domid,
> const char *mac, libxl_device_nic *nic)
> {
> @@ -53,8 +65,41 @@ int libxl_mac_to_device_nic(libxl_ctx *ctx, uint32_t domid,
> return rc;
> }
>
> +static int libxl__get_host_mac(libxl__gc *gc, unsigned char *buf)
> +{
> +int rc = ERROR_FAIL;
Having a blank line makes things a bit clearer.
> +#ifdef __linux__
> +struct ifaddrs *iface_list;
> +uint64_t largest = 0;
> +
> +if (getifaddrs(&iface_list) == 0) {
> +for (struct ifaddrs *iface = iface_list;
> +iface != NULL; iface = iface->ifa_next) {
> +if (iface->ifa_addr && iface->ifa_addr->sa_family == AF_PACKET) {
> +struct sockaddr_ll *s = (struct sockaddr_ll
> *)iface->ifa_addr;
Blank line here.
Generally please add a blank line between variable declarations and
statements.
> +if (s->sll_halen == 6) {
> +uint64_t value = 0;
Blank line here.
> +memcpy(&value, s->sll_addr, 6);
Please use sizeof(s->sll_addr) instead of 6.
Also please add an assert before the memcpy
assert(sizeof(value) >= sizeof(s->sll-addr));
This will help catch potential buffer overflow issue. Not that there is
one in your code, it is just good habit to have.
> +if (value > largest) {
> +memcpy(buf, s->sll_addr, 6);
> +largest = value;
> +rc = 0;
Interesting algorithm, but I don't know anything better at the moment.
:)
> +}
> +}
> +}
> +}
> +freeifaddrs(iface_list);
> +} else {
> +LOG(WARN, "getifaddrs\n");
Use LOGE here to log errno too. And please say
getifaddrs failed
And there is no need to have '\n' at the end of the line. LOG* macros
add that themselves.
> +}
> +#endif
> +
> +return rc;
> +}
> +
> static int libxl__device_nic_setdefault(libxl__gc *gc, uint32_t domid,
> -libxl_device_nic *nic, bool hotplug)
> +libxl_device_nic *nic, const char
> *name,
> +const int nic_index, bool hotplug)
> {
> int rc;
>
> @@ -65,11 +110,22 @@ static int libxl__device_nic_setdefault(libxl__gc *gc,
> uint32_t domid,
> if (!nic->model) return ERROR_NOMEM;
> }
> if (libxl__mac_is_default(&nic->mac)) {
> -const uint8_t *r;
> -libxl_uuid uuid;
> +uint8_t r[16];
> +
> +MD5_CTX ctx;
> +MD5Init(&ctx);
> +
> +uint8_t hostmac[6];
> +
> +if(libxl__get_host_mac(gc, hostmac) == 0) {
Missing a space after `if' here.
> +MD5Update(&ctx, hostmac, sizeof(hostmac));
> +} else {
> +LOGD(INFO, domid, "failed to get host mac address, will generate
> vm mac address without\n");
Remove '\n'.
Wei.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/3] xen/arm: vgic-v3: Bug fixes
> -Original Message- > From: Julien Grall [mailto:julien.gr...@arm.com] > Sent: 04 September 2018 20:22 > To: xen-devel@lists.xenproject.org > Cc: sstabell...@kernel.org; andre.przyw...@arm.com; Shameerali Kolothum > Thodi ; Julien Grall > > Subject: [PATCH 0/3] xen/arm: vgic-v3: Bug fixes > > Hi all, > > The first 2 patches of the series are meant to address Dom0 boot failure when > on GICv4 platforms as well as when the number of vCPUs is not equal to the > numbers of pCPUs. They should be backported up Xen 4.8. Tested first two patches on this series on ARM64 GICv4 platform(ACPI boot) and the reported issue[1] is no more present. Please feel free to add, Tested-by: Shameer Kolothum Thanks, Shameer [1] https://lists.xenproject.org/archives/html/xen-devel/2018-09/msg00138.html > Patch #3 should address failure when failing to create guest. ITS is still > under > EXPERT, so I would not consider it as backport. > > Cheers, > > Julien Grall (3): > [not-for-unstable] xen/arm: vgic-v3: Delay the initialization of the > domain information > xen/arm: vgic-v3: Don't create empty re-distributor regions > xen/arm: vgic-v3-its: Make vgic_v3_its_free_domain idempotent > > xen/arch/arm/gic-v3.c | 10 -- > xen/arch/arm/vgic-v3-its.c | 4 > xen/arch/arm/vgic-v3.c | 40 ++- > - > 3 files changed, 50 insertions(+), 4 deletions(-) > > -- > 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] x86 Community Call - Wed Sept 12, 14:00 - 15:00 UTC - Call for agenda items
Dear community members, please send me agenda items for next week’s community call by next Monday. One suggestion which was made to me recently by Rich, Christopher and Daniel was for Daniel to give an overview of Secure Boot, Measured Launch and TrenchBoot. @Daniel, @Christopher, @Rich: would this be possible during the next call? How much time does the talk require. I CC'ed attendees of the Arm call as sometimes there are cross-over issues which could be handled either in the Arm or x86 call. That way you get to see agenda, minutes, etc. and have a chance to decide on whether you want to join or not. Last month’s minutes are at https://docs.google.com/document/d/1zf9saqOvxAsIDvHPIFpzy56zsUdhu4LcHouQ0Uf2RGw/edit#heading=h.mz1wjb9vekjn Loose/ends: * Lars to bring up x86 bottleneck at next AB call – due to the Aug holidays we didn’t have any of the relevant vendors on the call * Andrew was going to look at L1TF related issues and add this to Jira (unless I misunderstood) See https://lists.xenproject.org/archives/html/xen-devel/2018-08/msg01160.html * Christopher will follow up on IRC/xen-devel@ re memory scrubbing * Juergen to send a mail related to 32PV support to xen-devel See https://lists.xenproject.org/archives/html/xen-devel/2018-08/msg01230.html Best Regards Lars ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] xen/balloon: add runtime control for scrubbing ballooned out pages
Scrubbing pages on initial balloon down can take some time, especially
in nested virtualization case (nested EPT is slow). When HVM/PVH guest is
started with memory= significantly lower than maxmem=, all the extra
pages will be scrubbed before returning to Xen. But since most of them
weren't used at all at that point, Xen needs to populate them first
(from populate-on-demand pool). In nested virt case (Xen inside KVM)
this slows down the guest boot by 15-30s with just 1.5GB needed to be
returned to Xen.
Add runtime parameter to enable/disable it, to allow initially disabling
scrubbing, then enable it back during boot (for example in initramfs).
Such usage relies on assumption that a) most pages ballooned out during
initial boot weren't used at all, and b) even if they were, very few
secrets are in the guest at that time (before any serious userspace
kicks in).
Default behaviour is unchanged.
Signed-off-by: Marek Marczykowski-Górecki
---
Is module_param() a good thing for this? Other xen-balloon parameters are
in /sys/devices/system/xen_memory, so maybe it would make sense to put
this one there too? But then, cmdline parameter would need to be added
separately and comment about core_param() suggests it shouldn't be used
if not absolutely necessary (is it?).
---
drivers/xen/Kconfig | 5 -
drivers/xen/mem-reservation.c | 7 +++
include/xen/mem-reservation.h | 11 ---
3 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index b459edfacff3..7b2c771e1813 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -87,7 +87,10 @@ config XEN_SCRUB_PAGES
Scrub pages before returning them to the system for reuse by
other domains. This makes sure that any confidential data
is not accidentally visible to other domains. Is it more
- secure, but slightly less efficient.
+ secure, but slightly less efficient. It can be disabled with
+ mem_reservation.xen_scrub_pages=0 and also controlled at runtime with
+ /sys/module/mem_reservation/parameters/xen_scrub_pages.
+
If in doubt, say yes.
config XEN_DEV_EVTCHN
diff --git a/drivers/xen/mem-reservation.c b/drivers/xen/mem-reservation.c
index 084799c6180e..5f08e19b6139 100644
--- a/drivers/xen/mem-reservation.c
+++ b/drivers/xen/mem-reservation.c
@@ -14,6 +14,13 @@
#include
#include
+#include
+
+#ifdef CONFIG_XEN_SCRUB_PAGES
+bool __read_mostly xen_scrub_pages = true;
+module_param(xen_scrub_pages, bool, 0644);
+MODULE_PARM_DESC(xen_scrub_pages, "Scrub ballooned pages before giving them
back to Xen");
+#endif
/*
* Use one extent per PAGE_SIZE to avoid to break down the page into
diff --git a/include/xen/mem-reservation.h b/include/xen/mem-reservation.h
index 80b52b4945e9..70c08f95bc84 100644
--- a/include/xen/mem-reservation.h
+++ b/include/xen/mem-reservation.h
@@ -17,12 +17,17 @@
#include
+#ifdef CONFIG_XEN_SCRUB_PAGES
+extern bool xen_scrub_pages;
+
static inline void xenmem_reservation_scrub_page(struct page *page)
{
-#ifdef CONFIG_XEN_SCRUB_PAGES
- clear_highpage(page);
-#endif
+ if (xen_scrub_pages)
+ clear_highpage(page);
}
+#else
+static inline void xenmem_reservation_scrub_page(struct page *page) { }
+#endif
#ifdef CONFIG_XEN_HAVE_PVMMU
void __xenmem_reservation_va_mapping_update(unsigned long count,
--
2.17.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/hvm/viridian: set shutdown_code in response to CrashNotify
> -Original Message- > From: Andrew Cooper > Sent: 06 September 2018 16:33 > To: Paul Durrant ; xen-devel@lists.xenproject.org > Cc: Jan Beulich > Subject: Re: [PATCH] x86/hvm/viridian: set shutdown_code in response to > CrashNotify > > On 06/09/18 16:32, Paul Durrant wrote: > >> -Original Message- > >> From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On > Behalf > >> Of Paul Durrant > >> Sent: 23 August 2018 09:38 > >> To: Andrew Cooper ; xen- > >> de...@lists.xenproject.org > >> Cc: Jan Beulich > >> Subject: Re: [Xen-devel] [PATCH] x86/hvm/viridian: set shutdown_code > in > >> response to CrashNotify > >> > >>> -Original Message- > >>> From: Andrew Cooper > >>> Sent: 16 August 2018 10:40 > >>> To: Paul Durrant ; xen- > >> de...@lists.xenproject.org > >>> Cc: Jan Beulich > >>> Subject: Re: [PATCH] x86/hvm/viridian: set shutdown_code in response > to > >>> CrashNotify > >>> > >>> On 10/08/18 16:59, Paul Durrant wrote: > > -Original Message- > > From: Andrew Cooper > > Sent: 10 August 2018 16:57 > > To: Paul Durrant ; xen- > >>> de...@lists.xenproject.org > > Cc: Jan Beulich > > Subject: Re: [PATCH] x86/hvm/viridian: set shutdown_code in > response > >>> to > > CrashNotify > > > > On 10/08/18 16:43, Paul Durrant wrote: > >> When Windows writes the CrashNotify bit in the CRASH_CTL MSR > then > >>> we > > know > >> it is crashing, so set the domain shutdown code appropriately. > >> > >> Signed-off-by: Paul Durrant > >> --- > >> Cc: Jan Beulich > >> Cc: Andrew Cooper > >> --- > >> xen/arch/x86/hvm/viridian.c | 4 > >> 1 file changed, 4 insertions(+) > >> > >> diff --git a/xen/arch/x86/hvm/viridian.c > b/xen/arch/x86/hvm/viridian.c > >> index 486065182c..294cf486cc 100644 > >> --- a/xen/arch/x86/hvm/viridian.c > >> +++ b/xen/arch/x86/hvm/viridian.c > >> @@ -645,6 +645,10 @@ int wrmsr_viridian_regs(uint32_t idx, > uint64_t > >>> val) > >> if ( !ctl.u.CrashNotify ) > >> break; > >> > >> +spin_lock(&d->shutdown_lock); > >> +d->shutdown_code = SHUTDOWN_crash; > >> +spin_unlock(&d->shutdown_lock); > > How does the domain eventually shut down? > I assume it shuts down when the guest writes to the PIIX. > > > It feels slightly odd to have > > a shutdown code before the domain has finished executing code. > > > That's the norm. The PV drivers (if they are installed) set it via a > schedop. > >>> This just makes sure it is set even if the PV drivers aren't there. > >>> > >>> What happens if the user has configured windows to reboot after a > crash? > >>> > >> That's exactly what this is trying to fix. Without the patch a VM without > >> PV > >> drivers will appear to just shut down (because nothing will set the > shutdown > >> code to anything else) so the actions-after-crash will not take effect. > >> > > Ping? Are you ok with the patch now? > > Lol - I was literally just trying to find it. I'll commit it in a moment. > :-) > ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/hvm/viridian: set shutdown_code in response to CrashNotify
On 06/09/18 16:32, Paul Durrant wrote: >> -Original Message- >> From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On Behalf >> Of Paul Durrant >> Sent: 23 August 2018 09:38 >> To: Andrew Cooper ; xen- >> de...@lists.xenproject.org >> Cc: Jan Beulich >> Subject: Re: [Xen-devel] [PATCH] x86/hvm/viridian: set shutdown_code in >> response to CrashNotify >> >>> -Original Message- >>> From: Andrew Cooper >>> Sent: 16 August 2018 10:40 >>> To: Paul Durrant ; xen- >> de...@lists.xenproject.org >>> Cc: Jan Beulich >>> Subject: Re: [PATCH] x86/hvm/viridian: set shutdown_code in response to >>> CrashNotify >>> >>> On 10/08/18 16:59, Paul Durrant wrote: > -Original Message- > From: Andrew Cooper > Sent: 10 August 2018 16:57 > To: Paul Durrant ; xen- >>> de...@lists.xenproject.org > Cc: Jan Beulich > Subject: Re: [PATCH] x86/hvm/viridian: set shutdown_code in response >>> to > CrashNotify > > On 10/08/18 16:43, Paul Durrant wrote: >> When Windows writes the CrashNotify bit in the CRASH_CTL MSR then >>> we > know >> it is crashing, so set the domain shutdown code appropriately. >> >> Signed-off-by: Paul Durrant >> --- >> Cc: Jan Beulich >> Cc: Andrew Cooper >> --- >> xen/arch/x86/hvm/viridian.c | 4 >> 1 file changed, 4 insertions(+) >> >> diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c >> index 486065182c..294cf486cc 100644 >> --- a/xen/arch/x86/hvm/viridian.c >> +++ b/xen/arch/x86/hvm/viridian.c >> @@ -645,6 +645,10 @@ int wrmsr_viridian_regs(uint32_t idx, uint64_t >>> val) >> if ( !ctl.u.CrashNotify ) >> break; >> >> +spin_lock(&d->shutdown_lock); >> +d->shutdown_code = SHUTDOWN_crash; >> +spin_unlock(&d->shutdown_lock); > How does the domain eventually shut down? I assume it shuts down when the guest writes to the PIIX. > It feels slightly odd to have > a shutdown code before the domain has finished executing code. > That's the norm. The PV drivers (if they are installed) set it via a schedop. >>> This just makes sure it is set even if the PV drivers aren't there. >>> >>> What happens if the user has configured windows to reboot after a crash? >>> >> That's exactly what this is trying to fix. Without the patch a VM without PV >> drivers will appear to just shut down (because nothing will set the shutdown >> code to anything else) so the actions-after-crash will not take effect. >> > Ping? Are you ok with the patch now? Lol - I was literally just trying to find it. I'll commit it in a moment. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/hvm/viridian: set shutdown_code in response to CrashNotify
> -Original Message- > From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On Behalf > Of Paul Durrant > Sent: 23 August 2018 09:38 > To: Andrew Cooper ; xen- > de...@lists.xenproject.org > Cc: Jan Beulich > Subject: Re: [Xen-devel] [PATCH] x86/hvm/viridian: set shutdown_code in > response to CrashNotify > > > -Original Message- > > From: Andrew Cooper > > Sent: 16 August 2018 10:40 > > To: Paul Durrant ; xen- > de...@lists.xenproject.org > > Cc: Jan Beulich > > Subject: Re: [PATCH] x86/hvm/viridian: set shutdown_code in response to > > CrashNotify > > > > On 10/08/18 16:59, Paul Durrant wrote: > > >> -Original Message- > > >> From: Andrew Cooper > > >> Sent: 10 August 2018 16:57 > > >> To: Paul Durrant ; xen- > > de...@lists.xenproject.org > > >> Cc: Jan Beulich > > >> Subject: Re: [PATCH] x86/hvm/viridian: set shutdown_code in response > > to > > >> CrashNotify > > >> > > >> On 10/08/18 16:43, Paul Durrant wrote: > > >>> When Windows writes the CrashNotify bit in the CRASH_CTL MSR then > > we > > >> know > > >>> it is crashing, so set the domain shutdown code appropriately. > > >>> > > >>> Signed-off-by: Paul Durrant > > >>> --- > > >>> Cc: Jan Beulich > > >>> Cc: Andrew Cooper > > >>> --- > > >>> xen/arch/x86/hvm/viridian.c | 4 > > >>> 1 file changed, 4 insertions(+) > > >>> > > >>> diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c > > >>> index 486065182c..294cf486cc 100644 > > >>> --- a/xen/arch/x86/hvm/viridian.c > > >>> +++ b/xen/arch/x86/hvm/viridian.c > > >>> @@ -645,6 +645,10 @@ int wrmsr_viridian_regs(uint32_t idx, uint64_t > > val) > > >>> if ( !ctl.u.CrashNotify ) > > >>> break; > > >>> > > >>> +spin_lock(&d->shutdown_lock); > > >>> +d->shutdown_code = SHUTDOWN_crash; > > >>> +spin_unlock(&d->shutdown_lock); > > >> How does the domain eventually shut down? > > > I assume it shuts down when the guest writes to the PIIX. > > > > > >> It feels slightly odd to have > > >> a shutdown code before the domain has finished executing code. > > >> > > > That's the norm. The PV drivers (if they are installed) set it via a > > > schedop. > > This just makes sure it is set even if the PV drivers aren't there. > > > > What happens if the user has configured windows to reboot after a crash? > > > > That's exactly what this is trying to fix. Without the patch a VM without PV > drivers will appear to just shut down (because nothing will set the shutdown > code to anything else) so the actions-after-crash will not take effect. > Ping? Are you ok with the patch now? Paul > Paul > > > ~Andrew > ___ > Xen-devel mailing list > Xen-devel@lists.xenproject.org > https://lists.xenproject.org/mailman/listinfo/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 07/16] x86/p2m/pod: make it build with !CONFIG_HVM
On 09/06/2018 11:57 AM, Wei Liu wrote: > On Tue, Sep 04, 2018 at 06:24:28PM +0100, Julien Grall wrote: >> Hi Wei, >> >> On 09/04/2018 05:15 PM, Wei Liu wrote: >>> Populate-on-demand is HVM only. >>> >>> Provide a bunch of stubs for common p2m code and guard one invocation >>> of guest_physmap_mark_populate_on_demand with is_hvm_domain. >>> >>> Put relevant fields in p2m_domain and code which touches those fields >>> under CONFIG_HVM. >> >> Arm does not have any POD support. Would it be worth to introduce a >> CONFIG_HAS_POD to avoid dummy function on Arm? > > That sounds fine. This will have the effect of not compiling PoD on Arm > at all, which is good. Another advantage of CONFIG_HAS_POD is that it would make it easier at some later point to compile out, if we decided that was worth doing -- I was going to suggest it just so as not to lose the "information" already contained in this patch as where all the PoD-related code was. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/5] x86/hvm: Make HVM_PARAM_{STORE, CONSOLE}_EVTCHN read-only to the guest
On 06/09/18 10:16, Paul Durrant wrote:
>> -Original Message-
>> From: Andrew Cooper [mailto:andrew.coop...@citrix.com]
>> Sent: 05 September 2018 19:12
>> To: Xen-devel
>> Cc: Andrew Cooper ; Jan Beulich
>> ; Wei Liu ; Roger Pau Monne
>> ; Paul Durrant ; Stefano
>> Stabellini ; Julien Grall
>> Subject: [PATCH 3/5] x86/hvm: Make
>> HVM_PARAM_{STORE,CONSOLE}_EVTCHN read-only to the guest
>>
>> These values are set by the toolstack for each create/restore operation, and
>> bound by xen{store,console}d before the the guest starts running.
>>
>> A guest has no reason to modify them at all, and the matching *_PFN
>> parameters
>> are already read-only. Adjust the *_EVTCHN permissions to be consistent.
> Unfortunately this patch will break the Windows PV driver function here:
>
> http://xenbits.xen.org/gitweb/?p=pvdrivers/win/xenbus.git;a=blob;f=src/xenbus/evtchn.c;hb=HEAD#l1037
>
> Unfortunately the values really do change across a reset. It would be
> possible to use volatile (disappear on reboot) registry keys to store the
> updated values instead but I don't really see any harm in allowing the guest
> to update the values to be correct, unless we want to change Xen to do the
> job so the guest doesn't have to go through this dance.
:( Everything is terrible.
This is a general problem, not x86 specific, so I'll drop this patch and
make a similar adjustment to the ARM one.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/5] x86/hvm: Switch hvm_allow_set_param() to use a whitelist
On 06/09/18 10:08, Paul Durrant wrote: >> -Original Message- >> From: Andrew Cooper [mailto:andrew.coop...@citrix.com] >> Sent: 05 September 2018 19:12 >> To: Xen-devel >> Cc: Andrew Cooper ; Jan Beulich >> ; Wei Liu ; Roger Pau Monne >> ; Paul Durrant ; Stefano >> Stabellini ; Julien Grall >> Subject: [PATCH 2/5] x86/hvm: Switch hvm_allow_set_param() to use a >> whitelist >> >> There are holes in the HVM_PARAM space, some of which are from >> deprecated >> parameters, but toolstack and device models currently has (almost) blanket > s/has/have > >> write access. >> >> Rearrange hvm_allow_get_param() to have a whitelist of toolstack-writeable > s/get/set Both fixed. >> d = rcu_lock_domain_by_any_id(a.domid); >> if ( d == NULL ) >> return -ESRCH; >> @@ -4209,15 +4228,7 @@ static int hvmop_set_param( >> case HVM_PARAM_ACPI_IOPORTS_LOCATION: >> rc = pmtimer_change_ioport(d, a.value); >> break; >> -case HVM_PARAM_MEMORY_EVENT_CR0: >> -case HVM_PARAM_MEMORY_EVENT_CR3: >> -case HVM_PARAM_MEMORY_EVENT_CR4: >> -case HVM_PARAM_MEMORY_EVENT_INT3: >> -case HVM_PARAM_MEMORY_EVENT_SINGLE_STEP: >> -case HVM_PARAM_MEMORY_EVENT_MSR: >> -/* Deprecated */ >> -rc = -EOPNOTSUPP; >> -break; > Does anything rely on this EOPNOTSUPP vs the EINVAL that would be returned > after this patch is applied? Nothing I can spot, although in searching, I see that xc_hvm_param_deprecated_check() is a thing. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/5] x86/hvm: Switch hvm_allow_get_param() to use a whitelist
On 06/09/18 09:56, Paul Durrant wrote:
>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>> index c22bf0b..96a6323 100644
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -4350,7 +4350,7 @@ static int hvm_allow_get_param(struct domain *d,
>>
>> switch ( a->index )
>> {
>> -/* The following parameters can be read by the guest. */
>> +/* The following parameters can be read by the guest and toolstack.
>> */
> Intentional indentation change? I guess so since you do it again below, but
> just checking.
Yes - this is where BSD style puts comments, as you are inside the
braced scope of the switch statement.
Notice furthermore that there is a correction to the end of the comment.
>
>> case HVM_PARAM_CALLBACK_IRQ:
>> case HVM_PARAM_VM86_TSS:
>> case HVM_PARAM_VM86_TSS_SIZED:
>> @@ -4363,18 +4363,39 @@ static int hvm_allow_get_param(struct domain
>> *d,
>> case HVM_PARAM_ALTP2M:
>> case HVM_PARAM_X87_FIP_WIDTH:
>> break;
>> -/*
>> - * The following parameters must not be read by the guest
>> - * since the domain may need to be paused.
>> - */
>> +
>> +/*
>> + * The following parameters are intended for toolstack usage only.
>> + * Some require the domain to be paused, and therefore may not read
>> by
>> + * the domain.
>> + */
>> +case HVM_PARAM_PAE_ENABLED:
>> case HVM_PARAM_IOREQ_PFN:
>> case HVM_PARAM_BUFIOREQ_PFN:
>> case HVM_PARAM_BUFIOREQ_EVTCHN:
>> -/* The remaining parameters should not be read by the guest. */
>> -default:
>> +case HVM_PARAM_VIRIDIAN:
>> +case HVM_PARAM_TIMER_MODE:
>> +case HVM_PARAM_HPET_ENABLED:
>> +case HVM_PARAM_IDENT_PT:
>> +case HVM_PARAM_DM_DOMAIN:
>> +case HVM_PARAM_ACPI_S_STATE:
>> +case HVM_PARAM_VPT_ALIGN:
>> +case HVM_PARAM_NESTEDHVM:
>> +case HVM_PARAM_PAGING_RING_PFN:
>> +case HVM_PARAM_MONITOR_RING_PFN:
>> +case HVM_PARAM_SHARING_RING_PFN:
>> +case HVM_PARAM_TRIPLE_FAULT_REASON:
>> +case HVM_PARAM_IOREQ_SERVER_PFN:
>> +case HVM_PARAM_NR_IOREQ_SERVER_PAGES:
>> +case HVM_PARAM_MCA_CAP:
>> if ( d == current->domain )
>> rc = -EPERM;
>> break;
>> +
>> +/* Hole, deprecated, or out-of-range. */
>> +default:
>> +rc = -EINVAL;
>> +break;
>> }
>>
>> return rc;
>> @@ -4390,9 +4411,6 @@ static int hvmop_get_param(
>> if ( copy_from_guest(&a, arg, 1) )
>> return -EFAULT;
>>
>> -if ( a.index >= HVM_NR_PARAMS )
>> -return -EINVAL;
>> -
> ASSERT, just in case someone screws up the allow function in future?
That's not going to help in any practical way. This check does really
exist, and is part of the switch statement.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 07/16] x86/p2m/pod: make it build with !CONFIG_HVM
>>> On 04.09.18 at 18:15, wrote:
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -52,15 +52,20 @@ DEFINE_PERCPU_RWLOCK_GLOBAL(p2m_percpu_rwlock);
> /* Init the datastructures for later use by the p2m code */
> static int p2m_initialise(struct domain *d, struct p2m_domain *p2m)
> {
> -unsigned int i;
> int ret = 0;
> +#ifdef CONFIG_HVM
> +unsigned int i;
> +#endif
>
> mm_rwlock_init(&p2m->lock);
> -mm_lock_init(&p2m->pod.lock);
> INIT_LIST_HEAD(&p2m->np2m_list);
> INIT_PAGE_LIST_HEAD(&p2m->pages);
> +
> +#ifdef CONFIG_HVM
> +mm_lock_init(&p2m->pod.lock);
> INIT_PAGE_LIST_HEAD(&p2m->pod.super);
> INIT_PAGE_LIST_HEAD(&p2m->pod.single);
> +#endif
>
> p2m->domain = d;
> p2m->default_access = p2m_access_rwx;
> @@ -69,8 +74,10 @@ static int p2m_initialise(struct domain *d, struct
> p2m_domain *p2m)
> p2m->np2m_base = P2M_BASE_EADDR;
> p2m->np2m_generation = 0;
>
> +#ifdef CONFIG_HVM
> for ( i = 0; i < ARRAY_SIZE(p2m->pod.mrp.list); ++i )
> p2m->pod.mrp.list[i] = gfn_x(INVALID_GFN);
> +#endif
I wonder whether all of this wouldn't better go into p2m_pod_init()
(or alike), to limit/avoid the #ifdef-ary here.
> @@ -2560,7 +2573,10 @@ void audit_p2m(struct domain *d,
> P2M_PRINTK("p2m audit starts\n");
>
> p2m_lock(p2m);
> +
> +#ifdef CONFIG_HVM
> pod_lock(p2m);
> +#endif
>
> if (p2m->audit_p2m)
> pmbad = p2m->audit_p2m(p2m);
> @@ -2621,7 +2637,9 @@ void audit_p2m(struct domain *d,
> }
> spin_unlock(&d->page_alloc_lock);
>
> +#ifdef CONFIG_HVM
> pod_unlock(p2m);
> +#endif
Perhaps better make them inline stubs? Otoh - isn't the whole
function useful for HVM only?
> --- a/xen/common/vm_event.c
> +++ b/xen/common/vm_event.c
> @@ -630,7 +630,9 @@ int vm_event_domctl(struct domain *d, struct
> xen_domctl_vm_event_op *vec,
> {
> case XEN_VM_EVENT_ENABLE:
> {
> +#ifdef CONFIG_HVM
> struct p2m_domain *p2m = p2m_get_hostp2m(d);
> +#endif
>
> rc = -EOPNOTSUPP;
> /* hvm fixme: p2m_is_foreign types need addressing */
> @@ -647,10 +649,12 @@ int vm_event_domctl(struct domain *d, struct
> xen_domctl_vm_event_op *vec,
> if ( unlikely(need_iommu(d)) )
> break;
>
> +#ifdef CONFIG_HVM
> rc = -EXDEV;
> /* Disallow paging in a PoD guest */
> if ( p2m->pod.entry_count )
> break;
> +#endif
Perhaps simply ditch the local variable?
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] automation: specify -j$(nproc) in build script
Signed-off-by: Wei Liu
---
automation/scripts/build | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/automation/scripts/build b/automation/scripts/build
index 054226bd73..c463b060d4 100755
--- a/automation/scripts/build
+++ b/automation/scripts/build
@@ -30,4 +30,4 @@ fi
./configure "${cfgargs[@]}"
-make dist
+make -j$(nproc) dist
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v6 01/14] iommu: introduce the concept of BFN...
> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 06 September 2018 14:13
> To: Paul Durrant
> Cc: Suravee Suthikulpanit ; Julien Grall
> ; Kevin Tian ; Stefano
> Stabellini ; xen-devel de...@lists.xenproject.org>
> Subject: RE: [Xen-devel] [PATCH v6 01/14] iommu: introduce the concept of
> BFN...
>
> >>> On 06.09.18 at 12:36, wrote:
> >> From: Jan Beulich [mailto:jbeul...@suse.com]
> >> Sent: 05 September 2018 10:39
> >>
> >> >>> On 05.09.18 at 11:13, wrote:
> >> > Personally I think 'bus address' is commonly enough used term for
> >> addresses
> >> > used by devices for DMA. Indeed we have already 'dev_bus_addr' in
> the
> >> grant
> >> > map and unmap hypercalls. So baddr and bfn seem like ok terms to me.
> It's
> >> > also not impossible to rename these later if they prove problematic.
> >>
> >> But that's the point - the names are problematic (to me): I permanently
> >> have to remind myself that they do _not_ refer to the addresses as
> >> seen when accessing memory, but the ones going _into_ the IOMMU.
> >
> > Ok. Could we agree on 'IOFN' then? I think 'iova' and 'io address' are also
> > reasonably widely used terms to refer to address from a device's PoV. I'd
> > really like to unblock these early patches.
>
> Hmm, earlier I had indicated I'd prefer DFN (as this make clear whose
> view we are talking about). Kevin seemed to prefer DFN too, just with
> a different association for D (which, as said, I consider unhelpful). So
> is there a particular reason you're now suggesting IOFN nevertheless?
It was the ambiguity and lack of agreement over the 'D' that made me think that
the other alternative would be better.
Kevin, would you be ok with 'IOFN'?
>
> >> The confusion (on my part) arises every time I see a mixture of gfn, bfn,
> >> and mfn in the same patch, perhaps including some 1:1-ness assumptions
> >> between pairs of them.
> >>
> >> Take these two hunks as example (mixing in some pfn as well):
> >>
> >> @@ -436,7 +436,7 @@ static int iommu_merge_pages(struct domain *d,
> >> unsigned long pt_mfn,
> >> * {Re, un}mapping super page frames causes re-allocation of io
> >> * page tables.
> >> */
> >> -static int iommu_pde_from_gfn(struct domain *d, unsigned long pfn,
> >> +static int iommu_pde_from_bfn(struct domain *d, unsigned long pfn,
> >>unsigned long pt_mfn[])
> >> {
> >> u64 *pde, *next_table_vaddr;
> >> @@ -477,11 +477,11 @@ static int iommu_pde_from_gfn(struct domain
> *d,
> >> unsigned long pfn,
> >> next_table_mfn != 0 )
> >> {
> >> int i;
> >> -unsigned long mfn, gfn;
> >> +unsigned long mfn, bfn;
> >> unsigned int page_sz;
> >>
> >> page_sz = 1 << (PTE_PER_TABLE_SHIFT * (next_level - 1));
> >> -gfn = pfn & ~((1 << (PTE_PER_TABLE_SHIFT * next_level)) - 1);
> >> +bfn = pfn & ~((1 << (PTE_PER_TABLE_SHIFT * next_level)) - 1);
> >
> > This is not wonderful code, agreed. In this particular case it looks like I
> > may be able to just rename the pfn argument to iofn (assuming we go with
> that
> > name) and lose the stack variable, if that helps.
>
> Renaming the parameter will likely help, I agree. Getting rid of the
> local variable, otoh, I'm not sure is going to work (you need to retain
> the function parameter's original value for the next iteration of the
> outer loop).
Oh, yes I missed that. I'll see what I can do once we have agreement on a name.
Paul
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-4.9 test] 127322: tolerable FAIL - PUSHED
flight 127322 linux-4.9 real [real] http://logs.test-lab.xenproject.org/osstest/logs/127322/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 127006 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 127006 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 127006 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 127006 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 127006 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-i386-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-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-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-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 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-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 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-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass version targeted for testing: linux9eabacaf4ce59a07baacac5f31586de4ae7e9194 baseline version: linuxe8d49e4292d9156a081752dee3f5a0cd12857da9 Last test of basis 127006 2018-08-30 19:38:22 Z6 days Testing same since 127298 2018-09-05 07:46:52 Z1 days2 attempts People who touched revisions under test: Alberto Panizzo Aleksander Morgado Alexander Sverdlin Alexander Usyskin Alexei Starovoitov Andi Kleen Andrew Morton Andrey Ryabinin Anthony Brandon Ariel Elior Arnd Bergmann Bart Van Assche Bernd Edlinger Bjørn Mork Calvin Walt
Re: [Xen-devel] [PATCH v3 1/3] x86/HVM: drop hvm_fetch_from_guest_linear()
> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 06 September 2018 14:03
> To: xen-devel
> Cc: Olaf Hering ; Andrew Cooper
> ; Paul Durrant ;
> Tim (Xen.org)
> Subject: [PATCH v3 1/3] x86/HVM: drop hvm_fetch_from_guest_linear()
>
> It can easily be expressed through hvm_copy_from_guest_linear(), and in
> two cases this even simplifies callers.
>
> Suggested-by: Paul Durrant
> Signed-off-by: Jan Beulich
> Reviewed-by: Andrew Cooper
> Tested-by: Olaf Hering
Reviewed-by: Paul Durrant
> ---
> v2: Make sure this compiles standalone. Slightly adjust change in
> hvm_ud_intercept().
>
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -1060,6 +1060,8 @@ static int __hvmemul_read(
> pfec |= PFEC_implicit;
> else if ( hvmemul_ctxt->seg_reg[x86_seg_ss].dpl == 3 )
> pfec |= PFEC_user_mode;
> +if ( access_type == hvm_access_insn_fetch )
> +pfec |= PFEC_insn_fetch;
>
> rc = hvmemul_virtual_to_linear(
> seg, offset, bytes, &reps, access_type, hvmemul_ctxt, &addr);
> @@ -1071,9 +1073,7 @@ static int __hvmemul_read(
> (vio->mmio_gla == (addr & PAGE_MASK)) )
> return hvmemul_linear_mmio_read(addr, bytes, p_data, pfec,
> hvmemul_ctxt, 1);
>
> -rc = ((access_type == hvm_access_insn_fetch) ?
> - hvm_fetch_from_guest_linear(p_data, addr, bytes, pfec, &pfinfo) :
> - hvm_copy_from_guest_linear(p_data, addr, bytes, pfec, &pfinfo));
> +rc = hvm_copy_from_guest_linear(p_data, addr, bytes, pfec, &pfinfo);
>
> switch ( rc )
> {
> @@ -2512,9 +2512,10 @@ void hvm_emulate_init_per_insn(
> hvm_access_insn_fetch,
> &hvmemul_ctxt->seg_reg[x86_seg_cs],
> &addr) &&
> - hvm_fetch_from_guest_linear(hvmemul_ctxt->insn_buf, addr,
> - sizeof(hvmemul_ctxt->insn_buf),
> - pfec, NULL) == HVMTRANS_okay) ?
> + hvm_copy_from_guest_linear(hvmemul_ctxt->insn_buf, addr,
> +sizeof(hvmemul_ctxt->insn_buf),
> +pfec | PFEC_insn_fetch,
> +NULL) == HVMTRANS_okay) ?
> sizeof(hvmemul_ctxt->insn_buf) : 0;
> }
> else
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -3286,15 +3286,6 @@ enum hvm_translation_result hvm_copy_fro
>PFEC_page_present | pfec, pfinfo);
> }
>
> -enum hvm_translation_result hvm_fetch_from_guest_linear(
> -void *buf, unsigned long addr, int size, uint32_t pfec,
> -pagefault_info_t *pfinfo)
> -{
> -return __hvm_copy(buf, addr, size, current,
> - HVMCOPY_from_guest | HVMCOPY_linear,
> - PFEC_page_present | PFEC_insn_fetch | pfec, pfinfo);
> -}
> -
> unsigned long copy_to_user_hvm(void *to, const void *from, unsigned int
> len)
> {
> int rc;
> @@ -3740,16 +3731,16 @@ void hvm_ud_intercept(struct cpu_user_re
> if ( opt_hvm_fep )
> {
> const struct segment_register *cs = &ctxt.seg_reg[x86_seg_cs];
> -uint32_t walk = (ctxt.seg_reg[x86_seg_ss].dpl == 3)
> -? PFEC_user_mode : 0;
> +uint32_t walk = ((ctxt.seg_reg[x86_seg_ss].dpl == 3)
> + ? PFEC_user_mode : 0) | PFEC_insn_fetch;
> unsigned long addr;
> char sig[5]; /* ud2; .ascii "xen" */
>
> if ( hvm_virtual_to_linear_addr(x86_seg_cs, cs, regs->rip,
> sizeof(sig), hvm_access_insn_fetch,
> cs, &addr) &&
> - (hvm_fetch_from_guest_linear(sig, addr, sizeof(sig),
> - walk, NULL) == HVMTRANS_okay) &&
> + (hvm_copy_from_guest_linear(sig, addr, sizeof(sig),
> + walk, NULL) == HVMTRANS_okay) &&
> (memcmp(sig, "\xf\xbxen", sizeof(sig)) == 0) )
> {
> regs->rip += sizeof(sig);
> --- a/xen/arch/x86/mm/shadow/common.c
> +++ b/xen/arch/x86/mm/shadow/common.c
> @@ -164,8 +164,9 @@ const struct x86_emulate_ops *shadow_ini
> (!hvm_translate_virtual_addr(
> x86_seg_cs, regs->rip, sizeof(sh_ctxt->insn_buf),
> hvm_access_insn_fetch, sh_ctxt, &addr) &&
> - !hvm_fetch_from_guest_linear(
> - sh_ctxt->insn_buf, addr, sizeof(sh_ctxt->insn_buf), 0, NULL))
> + !hvm_copy_from_guest_linear(
> + sh_ctxt->insn_buf, addr, sizeof(sh_ctxt->insn_buf),
> + PFEC_insn_fetch, NULL))
> ? sizeof(sh_ctxt->insn_buf) : 0;
>
> return &hvm_shadow_emulator_ops;
> @@ -198,8 +199,9 @@ void shadow_continue_emulation(struct sh
> (!hvm_translate_virtual_addr(
> x86
Re: [Xen-devel] [PATCH v3 2/3] x86/HVM: add known_gla() emulation helper
> -Original Message- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 06 September 2018 14:22 > To: Paul Durrant > Cc: Olaf Hering ; Andrew Cooper > ; xen-devel de...@lists.xenproject.org> > Subject: RE: [PATCH v3 2/3] x86/HVM: add known_gla() emulation helper > > >>> On 06.09.18 at 15:12, wrote: > >> -Original Message- > >> From: Jan Beulich [mailto:jbeul...@suse.com] > >> Sent: 06 September 2018 14:04 > >> To: xen-devel > >> Cc: Olaf Hering ; Andrew Cooper > >> ; Paul Durrant > >> Subject: [PATCH v3 2/3] x86/HVM: add known_gla() emulation helper > >> > >> ... as a central place to do respective checking for whether the > >> translation for the linear address is available as well as usable. > >> > >> Signed-off-by: Jan Beulich > >> --- > >> v3: Split from subsequent patch. > > > > Much nicer :-) > > > > Reviewed-by: Paul Durrant > > Thanks; any chance of also getting an ack for patch 1? > Oh sorry, I thought I already did... Coming up in a moment... Paul > Jan > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 05/16] x86: PIT emulation is common to both PV and HVM
>>> On 04.09.18 at 18:15, wrote: > Move the file to x86 common code and change its name to emul-i8254.c. > > Put HVM only code under CONFIG_HVM or is_hvm_domain. > > Signed-off-by: Wei Liu Acked-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] USB pass-through and WiFi access
Hi, 1. We are using Hikey960 board. 2. Xen version is 4.8 3. Linux kernel version is 4.14 We want to add/pass-through USB devices to DomU. From the below link I understood that there is no USB back and front driver support in latest kernel. But PVUSB support is added to *xl* tools since Xen 4.7. Please provide pointers on to achieve USB pass-through and hotplug. https://wiki.xenproject.org/wiki/Xen_USB_Passthrough Also we want share wireless internet between Dom0 and DomU. To achieve this what configuration we need to? -- Thanks & Regards VKS#436 -- This message contains confidential information and is intended only for the individual(s) named. If you are not the intended recipient, you are notified that disclosing, copying, distributing or taking any action in reliance on the contents of this mail and attached file/s is strictly prohibited. Please notify the sender immediately and delete this e-mail from your system. E-mail transmission cannot be guaranteed to be secured or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or contain viruses. The sender therefore does not accept liability for any errors or omissions in the contents of this message, which arise as a result of e-mail transmission. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] ARM: Xen 4.11 + modern dom0 on ODROID-XU4/HC1 (Exynos 5422)
On 01/09/18 09:45, Steve Dodd wrote: Hi all, Hello Steve, Having successfully followed the instructions at https://wiki.odroid.com/odroid-xu4/application_note/software/xen_virtualization to get an old Xen and dom0 kernel working on my ODROID HC1 I'd like to get a more modern setup working. I managed to get the 4.11 hypervisor to boot happily with the bits of https://github.com/bkrepo/xen/commit/8d56205455a4a1e0233421d3ee98e3c7dee20bd2 that deal with the CPU initialization (the trap/hypercall change seems to already be a part of 4.11 in a different form.) Any chance of getting this patch merged? AFAICT the patch can be divided in two: 1) Dom0 SMC forward While we did some work around trapping SMC, we still don't forward them directly to the firmware. If you need to forward Dom0 SMC call to the firmware, then we first need to know what they are used for. I would prefer a whilelisting over forward everything (even for Dom0). 2) SMP support for Odroid-XU This part would need to be cleaned up. We need to ensure the SMP bring-up is still working with the other exynos platform. Feel free to send patches on xen-devel, I would be happy to review them. What I am completely stuck on is getting a modern dom0 working. The handful of patches in https://github.com/bkrepo/linux-dom0/commits/odroidxu3-3.10.y-xen seem only to enable the relevant config options, patch the device tree with the arch timer and "xen-dom0" passthrough section, and backport the PSCI support which is already in 4.14, so I've tried applying these changes to the ODROID 4.14 tree at https://github.com/hardkernel/linux, with no success. Using the FDT from the working 3.x dom0 with the 4.14 zImage, it boots but doesn't seem to find any of the underlying hardware, certainly not the USB attached SSD which I need to boot. I would not recommend to use the 3.x FDT with 4.14. Despite the DT should be agnostic to the kernel revision, there are still often changes binding changes. Using the FDT from the ODROID 4.14 tree, patched for arch timer and the xen-dom0 section, the hypervisor sticks at "(XEN) 3..." I am not entirely sure to understand why you need to patch the arch timer. Could you send the diff of the FDT? I would also recommend to make sure the same configuration boots without Xen. This will tell you whether the bug is in Xen or Linux. Unfortunately I'm not at all familiar with either Xen or the ARM architecture, so would really appreciate it if anyone could steer me in the right direction! I hope I provided enough details. Feel free to ask any questions. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] xen/sched: Re-position the domain_update_node_affinity() call during vcpu construction
alloc_vcpu()'s call to domain_update_node_affinity() has existed for a decade,
but its effort is mostly wasted.
alloc_vcpu() is called in a loop for each vcpu, bringing them into existence.
The values of the affinity masks are still default, which is allcpus in
general, or a processor singleton for pinned domains.
Furthermore, domain_update_node_affinity() itself loops over all vcpus
accumulating the masks, making it a scalability concern with large numbers of
vcpus.
Move it to be called once after all vcpus are constructed, which has the same
net effect, but with fewer intermediate memory allocations and less cpumask
arithmetic.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
CC: Stefano Stabellini
CC: Julien Grall
CC: Dario Faggioli
This perhaps wants backporting to the maintenance trees, which is why I've
rebased it backwards over my other construction changes.
---
xen/arch/arm/domain_build.c | 2 ++
xen/arch/x86/hvm/dom0_build.c | 2 ++
xen/arch/x86/pv/dom0_build.c | 1 +
xen/common/domain.c | 3 ---
xen/common/domctl.c | 1 +
5 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 2a383c8..5389217 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2242,6 +2242,8 @@ int __init construct_dom0(struct domain *d)
vcpu_switch_to_aarch64_mode(d->vcpu[i]);
}
+domain_update_node_affinity(d);
+
v->is_initialised = 1;
clear_bit(_VPF_down, &v->pause_flags);
diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
index 22e335f..c63d7f0 100644
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -600,6 +600,8 @@ static int __init pvh_setup_cpus(struct domain *d, paddr_t
entry,
cpu = p->processor;
}
+domain_update_node_affinity(d);
+
rc = arch_set_info_hvm_guest(v, &cpu_ctx);
if ( rc )
{
diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
index 96ff0ee..44418b2 100644
--- a/xen/arch/x86/pv/dom0_build.c
+++ b/xen/arch/x86/pv/dom0_build.c
@@ -709,6 +709,7 @@ int __init dom0_construct_pv(struct domain *d,
cpu = p->processor;
}
+domain_update_node_affinity(d);
d->arch.paging.mode = 0;
/* Set up CR3 value for write_ptbase */
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 78c450e..6229ba7 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -193,9 +193,6 @@ struct vcpu *alloc_vcpu(
/* Must be called after making new vcpu visible to for_each_vcpu(). */
vcpu_check_shutdown(v);
-if ( !is_idle_domain(d) )
-domain_update_node_affinity(d);
-
return v;
}
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index ee0983d..faf26e7 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -590,6 +590,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t)
u_domctl)
goto maxvcpu_out;
}
+domain_update_node_affinity(d);
ret = 0;
maxvcpu_out:
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 02/16] x86: introduce and use a set of internal emulation flags
On Thu, Sep 06, 2018 at 07:27:47AM -0600, Jan Beulich wrote: > >>> On 04.09.18 at 18:15, wrote: > > Use these flags in has_* tests and emulation_flags_ok. > > > > Not using raw flags directly enabling DCE to kick in for has_* tests > > while at the same time making sure emulation_flags_ok won't go out of > > sync. > > This is rather hard to read (for me at least). Perhaps s/enabling/enables/ > and split sentences after the first line, or at least put a comma > somewhere? There were some grammar errors. The commit message has been updated to: Not using raw flags directly enables DCE to kick in for has_* tests, while at the same time makes sure emulation_flags_ok won't go out of sync. > > > Signed-off-by: Wei Liu > > Reviewed-by: Jan Beulich Thanks. Wei. > > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 03/16] x86: XENMEM_resource_ioreq_server is HVM only
>>> On 04.09.18 at 18:42, wrote: > On Tue, Sep 04, 2018 at 05:15:20PM +0100, Wei Liu wrote: >> Put the entire case branch under CONFIG_HVM. >> >> Nonetheless check HVM before trying to get ioreq server. > > The wording here is confusing. It should have been written as: > > Lift the check from hvm_get_ioreq_server_frame into its caller. And then Reviewed-by: Jan Beulich Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 02/16] x86: introduce and use a set of internal emulation flags
>>> On 04.09.18 at 18:15, wrote: > Use these flags in has_* tests and emulation_flags_ok. > > Not using raw flags directly enabling DCE to kick in for has_* tests > while at the same time making sure emulation_flags_ok won't go out of > sync. This is rather hard to read (for me at least). Perhaps s/enabling/enables/ and split sentences after the first line, or at least put a comma somewhere? > Signed-off-by: Wei Liu Reviewed-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 2/3] x86/HVM: add known_gla() emulation helper
>>> On 06.09.18 at 15:12, wrote: >> -Original Message- >> From: Jan Beulich [mailto:jbeul...@suse.com] >> Sent: 06 September 2018 14:04 >> To: xen-devel >> Cc: Olaf Hering ; Andrew Cooper >> ; Paul Durrant >> Subject: [PATCH v3 2/3] x86/HVM: add known_gla() emulation helper >> >> ... as a central place to do respective checking for whether the >> translation for the linear address is available as well as usable. >> >> Signed-off-by: Jan Beulich >> --- >> v3: Split from subsequent patch. > > Much nicer :-) > > Reviewed-by: Paul Durrant Thanks; any chance of also getting an ack for patch 1? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 3/3] x86/HVM: split page straddling emulated accesses in more cases
> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 06 September 2018 14:04
> To: xen-devel
> Cc: Olaf Hering ; Andrew Cooper
> ; Paul Durrant
> Subject: [PATCH v3 3/3] x86/HVM: split page straddling emulated accesses in
> more cases
>
> Assuming consecutive linear addresses map to all RAM or all MMIO is not
> correct. Nor is assuming that a page straddling MMIO access will access
> the same emulating component for both parts of the access. If a guest
> RAM read fails with HVMTRANS_bad_gfn_to_mfn and if the access straddles
> a page boundary, issue accesses separately for both parts.
>
> The extra call to known_gla() from hvmemul_write() is just to preserve
> original behavior; for consistency the check also gets added to
> hvmemul_rmw() (albeit I continue to be unsure whether we wouldn't better
> drop both).
>
> Note that the correctness of this depends on the MMIO caching used
> elsewhere in the emulation code.
>
> Signed-off-by: Jan Beulich
> Tested-by: Olaf Hering
Reviewed-by: Paul Durrant
> ---
> v3: Move introduction of known_gla() to a prereq patch. Mirror check
> using the function into hvmemul_rmw().
> v2: Also handle hvmemul_{write,rmw}().
>
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -1058,7 +1058,91 @@ static bool known_gla(unsigned long addr
> else if ( !vio->mmio_access.read_access )
> return false;
>
> -return vio->mmio_gla == (addr & PAGE_MASK);
> +return (vio->mmio_gla == (addr & PAGE_MASK) &&
> +(addr & ~PAGE_MASK) + bytes <= PAGE_SIZE);
> +}
> +
> +static int linear_read(unsigned long addr, unsigned int bytes, void *p_data,
> + uint32_t pfec, struct hvm_emulate_ctxt *hvmemul_ctxt)
> +{
> +pagefault_info_t pfinfo;
> +int rc = hvm_copy_from_guest_linear(p_data, addr, bytes, pfec,
> &pfinfo);
> +
> +switch ( rc )
> +{
> +unsigned int offset, part1;
> +
> +case HVMTRANS_okay:
> +return X86EMUL_OKAY;
> +
> +case HVMTRANS_bad_linear_to_gfn:
> +x86_emul_pagefault(pfinfo.ec, pfinfo.linear, &hvmemul_ctxt->ctxt);
> +return X86EMUL_EXCEPTION;
> +
> +case HVMTRANS_bad_gfn_to_mfn:
> +if ( pfec & PFEC_insn_fetch )
> +return X86EMUL_UNHANDLEABLE;
> +
> +offset = addr & ~PAGE_MASK;
> +if ( offset + bytes <= PAGE_SIZE )
> +return hvmemul_linear_mmio_read(addr, bytes, p_data, pfec,
> +hvmemul_ctxt,
> +known_gla(addr, bytes, pfec));
> +
> +/* Split the access at the page boundary. */
> +part1 = PAGE_SIZE - offset;
> +rc = linear_read(addr, part1, p_data, pfec, hvmemul_ctxt);
> +if ( rc == X86EMUL_OKAY )
> +rc = linear_read(addr + part1, bytes - part1, p_data + part1,
> + pfec, hvmemul_ctxt);
> +return rc;
> +
> +case HVMTRANS_gfn_paged_out:
> +case HVMTRANS_gfn_shared:
> +return X86EMUL_RETRY;
> +}
> +
> +return X86EMUL_UNHANDLEABLE;
> +}
> +
> +static int linear_write(unsigned long addr, unsigned int bytes, void *p_data,
> +uint32_t pfec, struct hvm_emulate_ctxt *hvmemul_ctxt)
> +{
> +pagefault_info_t pfinfo;
> +int rc = hvm_copy_to_guest_linear(addr, p_data, bytes, pfec, &pfinfo);
> +
> +switch ( rc )
> +{
> +unsigned int offset, part1;
> +
> +case HVMTRANS_okay:
> +return X86EMUL_OKAY;
> +
> +case HVMTRANS_bad_linear_to_gfn:
> +x86_emul_pagefault(pfinfo.ec, pfinfo.linear, &hvmemul_ctxt->ctxt);
> +return X86EMUL_EXCEPTION;
> +
> +case HVMTRANS_bad_gfn_to_mfn:
> +offset = addr & ~PAGE_MASK;
> +if ( offset + bytes <= PAGE_SIZE )
> +return hvmemul_linear_mmio_write(addr, bytes, p_data, pfec,
> + hvmemul_ctxt,
> + known_gla(addr, bytes, pfec));
> +
> +/* Split the access at the page boundary. */
> +part1 = PAGE_SIZE - offset;
> +rc = linear_write(addr, part1, p_data, pfec, hvmemul_ctxt);
> +if ( rc == X86EMUL_OKAY )
> +rc = linear_write(addr + part1, bytes - part1, p_data + part1,
> + pfec, hvmemul_ctxt);
> +return rc;
> +
> +case HVMTRANS_gfn_paged_out:
> +case HVMTRANS_gfn_shared:
> +return X86EMUL_RETRY;
> +}
> +
> +return X86EMUL_UNHANDLEABLE;
> }
>
> static int __hvmemul_read(
> @@ -1069,7 +1153,6 @@ static int __hvmemul_read(
> enum hvm_access_type access_type,
> struct hvm_emulate_ctxt *hvmemul_ctxt)
> {
> -pagefault_info_t pfinfo;
> unsigned long addr, reps = 1;
> uint32_t pfec = PFEC_page_present;
> int rc;
> @@ -1085,31 +1168,8 @@ static int __hvmemul_read(
> seg, offset, bytes, &reps, access_type, hvmemul_ctxt, &addr);
>
Re: [Xen-devel] [PATCH v6 01/14] iommu: introduce the concept of BFN...
>>> On 06.09.18 at 12:36, wrote:
>> From: Jan Beulich [mailto:jbeul...@suse.com]
>> Sent: 05 September 2018 10:39
>>
>> >>> On 05.09.18 at 11:13, wrote:
>> > Personally I think 'bus address' is commonly enough used term for
>> addresses
>> > used by devices for DMA. Indeed we have already 'dev_bus_addr' in the
>> grant
>> > map and unmap hypercalls. So baddr and bfn seem like ok terms to me. It's
>> > also not impossible to rename these later if they prove problematic.
>>
>> But that's the point - the names are problematic (to me): I permanently
>> have to remind myself that they do _not_ refer to the addresses as
>> seen when accessing memory, but the ones going _into_ the IOMMU.
>
> Ok. Could we agree on 'IOFN' then? I think 'iova' and 'io address' are also
> reasonably widely used terms to refer to address from a device's PoV. I'd
> really like to unblock these early patches.
Hmm, earlier I had indicated I'd prefer DFN (as this make clear whose
view we are talking about). Kevin seemed to prefer DFN too, just with
a different association for D (which, as said, I consider unhelpful). So
is there a particular reason you're now suggesting IOFN nevertheless?
>> The confusion (on my part) arises every time I see a mixture of gfn, bfn,
>> and mfn in the same patch, perhaps including some 1:1-ness assumptions
>> between pairs of them.
>>
>> Take these two hunks as example (mixing in some pfn as well):
>>
>> @@ -436,7 +436,7 @@ static int iommu_merge_pages(struct domain *d,
>> unsigned long pt_mfn,
>> * {Re, un}mapping super page frames causes re-allocation of io
>> * page tables.
>> */
>> -static int iommu_pde_from_gfn(struct domain *d, unsigned long pfn,
>> +static int iommu_pde_from_bfn(struct domain *d, unsigned long pfn,
>>unsigned long pt_mfn[])
>> {
>> u64 *pde, *next_table_vaddr;
>> @@ -477,11 +477,11 @@ static int iommu_pde_from_gfn(struct domain *d,
>> unsigned long pfn,
>> next_table_mfn != 0 )
>> {
>> int i;
>> -unsigned long mfn, gfn;
>> +unsigned long mfn, bfn;
>> unsigned int page_sz;
>>
>> page_sz = 1 << (PTE_PER_TABLE_SHIFT * (next_level - 1));
>> -gfn = pfn & ~((1 << (PTE_PER_TABLE_SHIFT * next_level)) - 1);
>> +bfn = pfn & ~((1 << (PTE_PER_TABLE_SHIFT * next_level)) - 1);
>
> This is not wonderful code, agreed. In this particular case it looks like I
> may be able to just rename the pfn argument to iofn (assuming we go with that
> name) and lose the stack variable, if that helps.
Renaming the parameter will likely help, I agree. Getting rid of the
local variable, otoh, I'm not sure is going to work (you need to retain
the function parameter's original value for the next iteration of the
outer loop).
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 2/3] x86/HVM: add known_gla() emulation helper
> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 06 September 2018 14:04
> To: xen-devel
> Cc: Olaf Hering ; Andrew Cooper
> ; Paul Durrant
> Subject: [PATCH v3 2/3] x86/HVM: add known_gla() emulation helper
>
> ... as a central place to do respective checking for whether the
> translation for the linear address is available as well as usable.
>
> Signed-off-by: Jan Beulich
> ---
> v3: Split from subsequent patch.
Much nicer :-)
Reviewed-by: Paul Durrant
>
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -1041,6 +1041,26 @@ static inline int hvmemul_linear_mmio_wr
>pfec, hvmemul_ctxt, translate);
> }
>
> +static bool known_gla(unsigned long addr, unsigned int bytes, uint32_t
> pfec)
> +{
> +const struct hvm_vcpu_io *vio = ¤t->arch.hvm.hvm_io;
> +
> +if ( pfec & PFEC_write_access )
> +{
> +if ( !vio->mmio_access.write_access )
> +return false;
> +}
> +else if ( pfec & PFEC_insn_fetch )
> +{
> +if ( !vio->mmio_access.insn_fetch )
> +return false;
> +}
> +else if ( !vio->mmio_access.read_access )
> +return false;
> +
> +return vio->mmio_gla == (addr & PAGE_MASK);
> +}
> +
> static int __hvmemul_read(
> enum x86_segment seg,
> unsigned long offset,
> @@ -1049,11 +1069,9 @@ static int __hvmemul_read(
> enum hvm_access_type access_type,
> struct hvm_emulate_ctxt *hvmemul_ctxt)
> {
> -struct vcpu *curr = current;
> pagefault_info_t pfinfo;
> unsigned long addr, reps = 1;
> uint32_t pfec = PFEC_page_present;
> -struct hvm_vcpu_io *vio = &curr->arch.hvm.hvm_io;
> int rc;
>
> if ( is_x86_system_segment(seg) )
> @@ -1067,10 +1085,7 @@ static int __hvmemul_read(
> seg, offset, bytes, &reps, access_type, hvmemul_ctxt, &addr);
> if ( rc != X86EMUL_OKAY || !bytes )
> return rc;
> -if ( ((access_type != hvm_access_insn_fetch
> - ? vio->mmio_access.read_access
> - : vio->mmio_access.insn_fetch)) &&
> - (vio->mmio_gla == (addr & PAGE_MASK)) )
> +if ( known_gla(addr, bytes, pfec) )
> return hvmemul_linear_mmio_read(addr, bytes, p_data, pfec,
> hvmemul_ctxt, 1);
>
> rc = hvm_copy_from_guest_linear(p_data, addr, bytes, pfec, &pfinfo);
> @@ -1171,10 +1186,8 @@ static int hvmemul_write(
> {
> struct hvm_emulate_ctxt *hvmemul_ctxt =
> container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
> -struct vcpu *curr = current;
> unsigned long addr, reps = 1;
> uint32_t pfec = PFEC_page_present | PFEC_write_access;
> -struct hvm_vcpu_io *vio = &curr->arch.hvm.hvm_io;
> int rc;
> void *mapping;
>
> @@ -1188,8 +1201,7 @@ static int hvmemul_write(
> if ( rc != X86EMUL_OKAY || !bytes )
> return rc;
>
> -if ( vio->mmio_access.write_access &&
> - (vio->mmio_gla == (addr & PAGE_MASK)) )
> +if ( known_gla(addr, bytes, pfec) )
> return hvmemul_linear_mmio_write(addr, bytes, p_data, pfec,
> hvmemul_ctxt, 1);
>
> mapping = hvmemul_map_linear_addr(addr, bytes, pfec, hvmemul_ctxt);
> @@ -1218,7 +1230,6 @@ static int hvmemul_rmw(
> container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
> unsigned long addr, reps = 1;
> uint32_t pfec = PFEC_page_present | PFEC_write_access;
> -struct hvm_vcpu_io *vio = ¤t->arch.hvm.hvm_io;
> int rc;
> void *mapping;
>
> @@ -1244,8 +1255,7 @@ static int hvmemul_rmw(
> else
> {
> unsigned long data = 0;
> -bool known_gpfn = vio->mmio_access.write_access &&
> - vio->mmio_gla == (addr & PAGE_MASK);
> +bool known_gpfn = known_gla(addr, bytes, pfec);
>
> if ( bytes > sizeof(data) )
> return X86EMUL_UNHANDLEABLE;
>
>
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] tools: specifically enable VirtFS in Linux QEMU builds
> -Original Message- > From: George Dunlap [mailto:george.dun...@citrix.com] > Sent: 06 September 2018 13:52 > To: Paul Durrant ; xen-devel@lists.xenproject.org > Cc: Andrew Cooper ; George Dunlap > ; Ian Jackson ; Jan > Beulich ; Julien Grall ; Konrad > Rzeszutek Wilk ; Stefano Stabellini > ; Tim (Xen.org) ; Wei Liu > > Subject: Re: [PATCH] tools: specifically enable VirtFS in Linux QEMU builds > > On 09/06/2018 01:06 PM, Paul Durrant wrote: > > 9pfs support has been a documented feature since Xen 4.9, but QEMU will > > not be built with backend support unless libcap and libattr dev packages > > are installed. > > > > This patch modifies the README to call out those packages as pre- > requisites > > for Linux builds and specifically enables VirtFS in the configure line > > for QEMU so that an error message is displayed if they are missing. > > > > Signed-off-by: Paul Durrant > > --- > > Cc: Andrew Cooper > > Cc: George Dunlap > > Cc: Ian Jackson > > Cc: Jan Beulich > > Cc: Julien Grall > > Cc: Konrad Rzeszutek Wilk > > Cc: Stefano Stabellini > > Cc: Tim Deegan > > Cc: Wei Liu > > --- > > README | 2 ++ > > tools/Makefile | 11 +++ > > 2 files changed, 13 insertions(+) > > > > diff --git a/README b/README > > index 4b95b21c7b..1a4e4b2c1b 100644 > > --- a/README > > +++ b/README > > @@ -56,6 +56,8 @@ provided by your OS distributor: > >greater. > > * Development install of GLib v2.0 (e.g. libglib2.0-dev) > > * Development install of Pixman (e.g. libpixman-1-dev) > > +* Development install of libcap (e.g. libcap-dev) [Linux only] > > +* Development install of libattr (e.g. libattr1-dev) [Linux only] > > * pkg-config > > * bridge-utils package (/sbin/brctl) > > * iproute package (/sbin/ip) > > diff --git a/tools/Makefile b/tools/Makefile > > index 67977ad850..e74efb8a6e 100644 > > --- a/tools/Makefile > > +++ b/tools/Makefile > > @@ -216,6 +216,11 @@ else > > QEMU_XEN_ENABLE_DEBUG := > > endif > > > > +# > > +# 9pfs support is a documented feature but it depends on a QEMU with > > +# VirtFS enabled. However VirtFS is a Linux-only option so only enable > > +# it for Linux builds. > > +# > > subdir-all-qemu-xen-dir: qemu-xen-dir-find > > unset MAKELEVEL; \ > > if test -d $(QEMU_UPSTREAM_LOC) ; then \ > > @@ -232,10 +237,16 @@ subdir-all-qemu-xen-dir: qemu-xen-dir-find > > else \ > > enable_trace_backend='' ; \ > > fi ; \ > > + if [ "$(CONFIG_Linux)" = "y" ]; then \ > > + enable_virtfs='--enable-virtfs' ; \ > > + else \ > > + enable_virtfs='' ; \ > > + fi ; \ > > 9pfs is still a reasonably "niche" feature; I don't think we want to > force people to enable it if they don't expect to use it. > > Is it the case that if we have libcap and libattr, that qemu will enable > 9pfs automatically? If so we should just document that. Yes, that is the case but it does seem weird that we have a documented feature which is not being built in by default, because we don't call out or check for the necessary pre-requisites. > > Another option would be to have "--enable-9pfs" option in the tools > configure, which would then pass this on to qemu (and thus fail if the > requisite librares aren't present), but I'm not sure if having a load of > "--enable-*" at the top level is really what we want for this. > That is something I wondered about. Really I just don't want people to be scrobbling around wondering why, having built and installed Xen and QEMU put a '9p' line in their config and then kicked off their guest, it simply doesn't work. Paul > -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 3/3] x86/HVM: split page straddling emulated accesses in more cases
Assuming consecutive linear addresses map to all RAM or all MMIO is not
correct. Nor is assuming that a page straddling MMIO access will access
the same emulating component for both parts of the access. If a guest
RAM read fails with HVMTRANS_bad_gfn_to_mfn and if the access straddles
a page boundary, issue accesses separately for both parts.
The extra call to known_gla() from hvmemul_write() is just to preserve
original behavior; for consistency the check also gets added to
hvmemul_rmw() (albeit I continue to be unsure whether we wouldn't better
drop both).
Note that the correctness of this depends on the MMIO caching used
elsewhere in the emulation code.
Signed-off-by: Jan Beulich
Tested-by: Olaf Hering
---
v3: Move introduction of known_gla() to a prereq patch. Mirror check
using the function into hvmemul_rmw().
v2: Also handle hvmemul_{write,rmw}().
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -1058,7 +1058,91 @@ static bool known_gla(unsigned long addr
else if ( !vio->mmio_access.read_access )
return false;
-return vio->mmio_gla == (addr & PAGE_MASK);
+return (vio->mmio_gla == (addr & PAGE_MASK) &&
+(addr & ~PAGE_MASK) + bytes <= PAGE_SIZE);
+}
+
+static int linear_read(unsigned long addr, unsigned int bytes, void *p_data,
+ uint32_t pfec, struct hvm_emulate_ctxt *hvmemul_ctxt)
+{
+pagefault_info_t pfinfo;
+int rc = hvm_copy_from_guest_linear(p_data, addr, bytes, pfec, &pfinfo);
+
+switch ( rc )
+{
+unsigned int offset, part1;
+
+case HVMTRANS_okay:
+return X86EMUL_OKAY;
+
+case HVMTRANS_bad_linear_to_gfn:
+x86_emul_pagefault(pfinfo.ec, pfinfo.linear, &hvmemul_ctxt->ctxt);
+return X86EMUL_EXCEPTION;
+
+case HVMTRANS_bad_gfn_to_mfn:
+if ( pfec & PFEC_insn_fetch )
+return X86EMUL_UNHANDLEABLE;
+
+offset = addr & ~PAGE_MASK;
+if ( offset + bytes <= PAGE_SIZE )
+return hvmemul_linear_mmio_read(addr, bytes, p_data, pfec,
+hvmemul_ctxt,
+known_gla(addr, bytes, pfec));
+
+/* Split the access at the page boundary. */
+part1 = PAGE_SIZE - offset;
+rc = linear_read(addr, part1, p_data, pfec, hvmemul_ctxt);
+if ( rc == X86EMUL_OKAY )
+rc = linear_read(addr + part1, bytes - part1, p_data + part1,
+ pfec, hvmemul_ctxt);
+return rc;
+
+case HVMTRANS_gfn_paged_out:
+case HVMTRANS_gfn_shared:
+return X86EMUL_RETRY;
+}
+
+return X86EMUL_UNHANDLEABLE;
+}
+
+static int linear_write(unsigned long addr, unsigned int bytes, void *p_data,
+uint32_t pfec, struct hvm_emulate_ctxt *hvmemul_ctxt)
+{
+pagefault_info_t pfinfo;
+int rc = hvm_copy_to_guest_linear(addr, p_data, bytes, pfec, &pfinfo);
+
+switch ( rc )
+{
+unsigned int offset, part1;
+
+case HVMTRANS_okay:
+return X86EMUL_OKAY;
+
+case HVMTRANS_bad_linear_to_gfn:
+x86_emul_pagefault(pfinfo.ec, pfinfo.linear, &hvmemul_ctxt->ctxt);
+return X86EMUL_EXCEPTION;
+
+case HVMTRANS_bad_gfn_to_mfn:
+offset = addr & ~PAGE_MASK;
+if ( offset + bytes <= PAGE_SIZE )
+return hvmemul_linear_mmio_write(addr, bytes, p_data, pfec,
+ hvmemul_ctxt,
+ known_gla(addr, bytes, pfec));
+
+/* Split the access at the page boundary. */
+part1 = PAGE_SIZE - offset;
+rc = linear_write(addr, part1, p_data, pfec, hvmemul_ctxt);
+if ( rc == X86EMUL_OKAY )
+rc = linear_write(addr + part1, bytes - part1, p_data + part1,
+ pfec, hvmemul_ctxt);
+return rc;
+
+case HVMTRANS_gfn_paged_out:
+case HVMTRANS_gfn_shared:
+return X86EMUL_RETRY;
+}
+
+return X86EMUL_UNHANDLEABLE;
}
static int __hvmemul_read(
@@ -1069,7 +1153,6 @@ static int __hvmemul_read(
enum hvm_access_type access_type,
struct hvm_emulate_ctxt *hvmemul_ctxt)
{
-pagefault_info_t pfinfo;
unsigned long addr, reps = 1;
uint32_t pfec = PFEC_page_present;
int rc;
@@ -1085,31 +1168,8 @@ static int __hvmemul_read(
seg, offset, bytes, &reps, access_type, hvmemul_ctxt, &addr);
if ( rc != X86EMUL_OKAY || !bytes )
return rc;
-if ( known_gla(addr, bytes, pfec) )
-return hvmemul_linear_mmio_read(addr, bytes, p_data, pfec,
hvmemul_ctxt, 1);
-rc = hvm_copy_from_guest_linear(p_data, addr, bytes, pfec, &pfinfo);
-
-switch ( rc )
-{
-case HVMTRANS_okay:
-break;
-case HVMTRANS_bad_linear_to_gfn:
-x86_emul_pagefault(pfinfo.ec, pfinfo.linear, &hvmemul_ctxt->ctxt);
-return X86EMUL_EXCEPTION;
-case HVMTRANS_bad_gfn_to_mfn:
-if ( access_type == hvm_access_
[Xen-devel] [PATCH v3 2/3] x86/HVM: add known_gla() emulation helper
... as a central place to do respective checking for whether the
translation for the linear address is available as well as usable.
Signed-off-by: Jan Beulich
---
v3: Split from subsequent patch.
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -1041,6 +1041,26 @@ static inline int hvmemul_linear_mmio_wr
pfec, hvmemul_ctxt, translate);
}
+static bool known_gla(unsigned long addr, unsigned int bytes, uint32_t pfec)
+{
+const struct hvm_vcpu_io *vio = ¤t->arch.hvm.hvm_io;
+
+if ( pfec & PFEC_write_access )
+{
+if ( !vio->mmio_access.write_access )
+return false;
+}
+else if ( pfec & PFEC_insn_fetch )
+{
+if ( !vio->mmio_access.insn_fetch )
+return false;
+}
+else if ( !vio->mmio_access.read_access )
+return false;
+
+return vio->mmio_gla == (addr & PAGE_MASK);
+}
+
static int __hvmemul_read(
enum x86_segment seg,
unsigned long offset,
@@ -1049,11 +1069,9 @@ static int __hvmemul_read(
enum hvm_access_type access_type,
struct hvm_emulate_ctxt *hvmemul_ctxt)
{
-struct vcpu *curr = current;
pagefault_info_t pfinfo;
unsigned long addr, reps = 1;
uint32_t pfec = PFEC_page_present;
-struct hvm_vcpu_io *vio = &curr->arch.hvm.hvm_io;
int rc;
if ( is_x86_system_segment(seg) )
@@ -1067,10 +1085,7 @@ static int __hvmemul_read(
seg, offset, bytes, &reps, access_type, hvmemul_ctxt, &addr);
if ( rc != X86EMUL_OKAY || !bytes )
return rc;
-if ( ((access_type != hvm_access_insn_fetch
- ? vio->mmio_access.read_access
- : vio->mmio_access.insn_fetch)) &&
- (vio->mmio_gla == (addr & PAGE_MASK)) )
+if ( known_gla(addr, bytes, pfec) )
return hvmemul_linear_mmio_read(addr, bytes, p_data, pfec,
hvmemul_ctxt, 1);
rc = hvm_copy_from_guest_linear(p_data, addr, bytes, pfec, &pfinfo);
@@ -1171,10 +1186,8 @@ static int hvmemul_write(
{
struct hvm_emulate_ctxt *hvmemul_ctxt =
container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
-struct vcpu *curr = current;
unsigned long addr, reps = 1;
uint32_t pfec = PFEC_page_present | PFEC_write_access;
-struct hvm_vcpu_io *vio = &curr->arch.hvm.hvm_io;
int rc;
void *mapping;
@@ -1188,8 +1201,7 @@ static int hvmemul_write(
if ( rc != X86EMUL_OKAY || !bytes )
return rc;
-if ( vio->mmio_access.write_access &&
- (vio->mmio_gla == (addr & PAGE_MASK)) )
+if ( known_gla(addr, bytes, pfec) )
return hvmemul_linear_mmio_write(addr, bytes, p_data, pfec,
hvmemul_ctxt, 1);
mapping = hvmemul_map_linear_addr(addr, bytes, pfec, hvmemul_ctxt);
@@ -1218,7 +1230,6 @@ static int hvmemul_rmw(
container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
unsigned long addr, reps = 1;
uint32_t pfec = PFEC_page_present | PFEC_write_access;
-struct hvm_vcpu_io *vio = ¤t->arch.hvm.hvm_io;
int rc;
void *mapping;
@@ -1244,8 +1255,7 @@ static int hvmemul_rmw(
else
{
unsigned long data = 0;
-bool known_gpfn = vio->mmio_access.write_access &&
- vio->mmio_gla == (addr & PAGE_MASK);
+bool known_gpfn = known_gla(addr, bytes, pfec);
if ( bytes > sizeof(data) )
return X86EMUL_UNHANDLEABLE;
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 1/3] x86/HVM: drop hvm_fetch_from_guest_linear()
It can easily be expressed through hvm_copy_from_guest_linear(), and in
two cases this even simplifies callers.
Suggested-by: Paul Durrant
Signed-off-by: Jan Beulich
Reviewed-by: Andrew Cooper
Tested-by: Olaf Hering
---
v2: Make sure this compiles standalone. Slightly adjust change in
hvm_ud_intercept().
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -1060,6 +1060,8 @@ static int __hvmemul_read(
pfec |= PFEC_implicit;
else if ( hvmemul_ctxt->seg_reg[x86_seg_ss].dpl == 3 )
pfec |= PFEC_user_mode;
+if ( access_type == hvm_access_insn_fetch )
+pfec |= PFEC_insn_fetch;
rc = hvmemul_virtual_to_linear(
seg, offset, bytes, &reps, access_type, hvmemul_ctxt, &addr);
@@ -1071,9 +1073,7 @@ static int __hvmemul_read(
(vio->mmio_gla == (addr & PAGE_MASK)) )
return hvmemul_linear_mmio_read(addr, bytes, p_data, pfec,
hvmemul_ctxt, 1);
-rc = ((access_type == hvm_access_insn_fetch) ?
- hvm_fetch_from_guest_linear(p_data, addr, bytes, pfec, &pfinfo) :
- hvm_copy_from_guest_linear(p_data, addr, bytes, pfec, &pfinfo));
+rc = hvm_copy_from_guest_linear(p_data, addr, bytes, pfec, &pfinfo);
switch ( rc )
{
@@ -2512,9 +2512,10 @@ void hvm_emulate_init_per_insn(
hvm_access_insn_fetch,
&hvmemul_ctxt->seg_reg[x86_seg_cs],
&addr) &&
- hvm_fetch_from_guest_linear(hvmemul_ctxt->insn_buf, addr,
- sizeof(hvmemul_ctxt->insn_buf),
- pfec, NULL) == HVMTRANS_okay) ?
+ hvm_copy_from_guest_linear(hvmemul_ctxt->insn_buf, addr,
+sizeof(hvmemul_ctxt->insn_buf),
+pfec | PFEC_insn_fetch,
+NULL) == HVMTRANS_okay) ?
sizeof(hvmemul_ctxt->insn_buf) : 0;
}
else
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3286,15 +3286,6 @@ enum hvm_translation_result hvm_copy_fro
PFEC_page_present | pfec, pfinfo);
}
-enum hvm_translation_result hvm_fetch_from_guest_linear(
-void *buf, unsigned long addr, int size, uint32_t pfec,
-pagefault_info_t *pfinfo)
-{
-return __hvm_copy(buf, addr, size, current,
- HVMCOPY_from_guest | HVMCOPY_linear,
- PFEC_page_present | PFEC_insn_fetch | pfec, pfinfo);
-}
-
unsigned long copy_to_user_hvm(void *to, const void *from, unsigned int len)
{
int rc;
@@ -3740,16 +3731,16 @@ void hvm_ud_intercept(struct cpu_user_re
if ( opt_hvm_fep )
{
const struct segment_register *cs = &ctxt.seg_reg[x86_seg_cs];
-uint32_t walk = (ctxt.seg_reg[x86_seg_ss].dpl == 3)
-? PFEC_user_mode : 0;
+uint32_t walk = ((ctxt.seg_reg[x86_seg_ss].dpl == 3)
+ ? PFEC_user_mode : 0) | PFEC_insn_fetch;
unsigned long addr;
char sig[5]; /* ud2; .ascii "xen" */
if ( hvm_virtual_to_linear_addr(x86_seg_cs, cs, regs->rip,
sizeof(sig), hvm_access_insn_fetch,
cs, &addr) &&
- (hvm_fetch_from_guest_linear(sig, addr, sizeof(sig),
- walk, NULL) == HVMTRANS_okay) &&
+ (hvm_copy_from_guest_linear(sig, addr, sizeof(sig),
+ walk, NULL) == HVMTRANS_okay) &&
(memcmp(sig, "\xf\xbxen", sizeof(sig)) == 0) )
{
regs->rip += sizeof(sig);
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -164,8 +164,9 @@ const struct x86_emulate_ops *shadow_ini
(!hvm_translate_virtual_addr(
x86_seg_cs, regs->rip, sizeof(sh_ctxt->insn_buf),
hvm_access_insn_fetch, sh_ctxt, &addr) &&
- !hvm_fetch_from_guest_linear(
- sh_ctxt->insn_buf, addr, sizeof(sh_ctxt->insn_buf), 0, NULL))
+ !hvm_copy_from_guest_linear(
+ sh_ctxt->insn_buf, addr, sizeof(sh_ctxt->insn_buf),
+ PFEC_insn_fetch, NULL))
? sizeof(sh_ctxt->insn_buf) : 0;
return &hvm_shadow_emulator_ops;
@@ -198,8 +199,9 @@ void shadow_continue_emulation(struct sh
(!hvm_translate_virtual_addr(
x86_seg_cs, regs->rip, sizeof(sh_ctxt->insn_buf),
hvm_access_insn_fetch, sh_ctxt, &addr) &&
- !hvm_fetch_from_guest_linear(
- sh_ctxt->insn_buf, addr, sizeof(sh_ctxt->insn_buf), 0, NULL))
+ !hvm_copy_from_guest_linear(
+ sh_ctxt->insn_buf, addr, sizeof(sh_ctxt->insn_buf),
+ PFEC_insn_fetch, NULL))
? sizeof(sh_ctxt->insn_buf) : 0;
sh_ctxt->insn_buf_eip = regs->rip;
}
--- a/xen/arch/x86
Re: [Xen-devel] [PATCH] xen/manage: don't complain about an empty value in control/sysrq node
On Thu, Sep 06, 2018 at 01:26:08PM +0200, Vitaly Kuznetsov wrote: > When guest receives a sysrq request from the host it acknowledges it by > writing '\0' to control/sysrq xenstore node. This, however, make xenstore > watch fire again but xenbus_scanf() fails to parse empty value with "%c" > format string: > > sysrq: SysRq : Emergency Sync > Emergency Sync complete > xen:manage: Error -34 reading sysrq code in control/sysrq > > Ignore -ERANGE the same way we already ignore -ENOENT, empty value in > control/sysrq is totally legal. > > Signed-off-by: Vitaly Kuznetsov Reviewed-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 0/3] x86/HVM: emulation adjustments
1: drop hvm_fetch_from_guest_linear() 2: add known_gla() emulation helper 3: split page straddling emulated accesses in more cases v3: Split patch 2 out from patch 3. v2: Patch 1 now builds cleanly (without other patches in place the up-to- date versions of which are yet to be posted), and patch 2 is no longer RFC. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] tools: specifically enable VirtFS in Linux QEMU builds
On 09/06/2018 01:06 PM, Paul Durrant wrote: > 9pfs support has been a documented feature since Xen 4.9, but QEMU will > not be built with backend support unless libcap and libattr dev packages > are installed. > > This patch modifies the README to call out those packages as pre-requisites > for Linux builds and specifically enables VirtFS in the configure line > for QEMU so that an error message is displayed if they are missing. > > Signed-off-by: Paul Durrant > --- > Cc: Andrew Cooper > Cc: George Dunlap > Cc: Ian Jackson > Cc: Jan Beulich > Cc: Julien Grall > Cc: Konrad Rzeszutek Wilk > Cc: Stefano Stabellini > Cc: Tim Deegan > Cc: Wei Liu > --- > README | 2 ++ > tools/Makefile | 11 +++ > 2 files changed, 13 insertions(+) > > diff --git a/README b/README > index 4b95b21c7b..1a4e4b2c1b 100644 > --- a/README > +++ b/README > @@ -56,6 +56,8 @@ provided by your OS distributor: >greater. > * Development install of GLib v2.0 (e.g. libglib2.0-dev) > * Development install of Pixman (e.g. libpixman-1-dev) > +* Development install of libcap (e.g. libcap-dev) [Linux only] > +* Development install of libattr (e.g. libattr1-dev) [Linux only] > * pkg-config > * bridge-utils package (/sbin/brctl) > * iproute package (/sbin/ip) > diff --git a/tools/Makefile b/tools/Makefile > index 67977ad850..e74efb8a6e 100644 > --- a/tools/Makefile > +++ b/tools/Makefile > @@ -216,6 +216,11 @@ else > QEMU_XEN_ENABLE_DEBUG := > endif > > +# > +# 9pfs support is a documented feature but it depends on a QEMU with > +# VirtFS enabled. However VirtFS is a Linux-only option so only enable > +# it for Linux builds. > +# > subdir-all-qemu-xen-dir: qemu-xen-dir-find > unset MAKELEVEL; \ > if test -d $(QEMU_UPSTREAM_LOC) ; then \ > @@ -232,10 +237,16 @@ subdir-all-qemu-xen-dir: qemu-xen-dir-find > else \ > enable_trace_backend='' ; \ > fi ; \ > + if [ "$(CONFIG_Linux)" = "y" ]; then \ > + enable_virtfs='--enable-virtfs' ; \ > + else \ > + enable_virtfs='' ; \ > + fi ; \ 9pfs is still a reasonably "niche" feature; I don't think we want to force people to enable it if they don't expect to use it. Is it the case that if we have libcap and libattr, that qemu will enable 9pfs automatically? If so we should just document that. Another option would be to have "--enable-9pfs" option in the tools configure, which would then pass this on to qemu (and thus fail if the requisite librares aren't present), but I'm not sure if having a load of "--enable-*" at the top level is really what we want for this. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Automatic dependencies are out of sync
On 06/09/18 12:26, Jan Beulich wrote: On 06.09.18 at 10:27, wrote: >> On 06/09/18 10:10, Jan Beulich wrote: >> On 06.09.18 at 09:34, wrote: I've setup a little example Makefile solving the problem (just to show the correct dependencies, needs to be adapted for naming the .d and .d2 files and how to build the .d2): -->8 snip here 8<-- DEPS := tst.d2 all: tst $(DEPS) >>> >>> -include $(DEPS) already ought to have the effect of such a dependency, >>> since all makefiles are checked for rules of how to re-make them. >> >> Obviously this isn't the case. Otherwise there would be .d2 files more >> common after doing a make. > > Well, be this (mis?)behavior is what needs explaining first. In my example Makefile this is working as you are expecting it: even with removing the dependency of "all" on $(DEPS) everything is fine. I suspect there are no .d2 files after the first make due to: DEPS = .*.d DEPS_INCLUDE = $(addsuffix .d2, $(basename $(wildcard $(DEPS In the beginning there is no .d file, so DEPS_INCLUDE will be empty. > %.o %.d: %.c gcc -MMD -o $(patsubst %.c,%.o,$<) -c $< >>> >>> Doesn't this result in gcc to be invoked twice, perhaps resulting in >>> corrupt .o and/or .d? I think %.d wants to depend on %.o, without >>> a command. >> >> No, that's perfectly fine. make will invoke the command only once, its >> just not clear for which target (that's the reason I need to use the >> $(patsubst %.c,%.o,$<) instead of $@, which might be the .o _or_ the .d >> file). >> >> From the make docs: >> >> Pattern rules may have more than one target. Unlike normal rules, this >> does not act as many different rules with the same prerequisites and >> recipe. If a pattern rule has multiple targets, make knows that the >> rule’s recipe is responsible for making all of the targets. The recipe >> is executed only once to make all the targets. > > Oh, right. But then there's no need to play games - just use $*. Aah, right. Good idea. > %: %.o gcc $< -o $@ -include $(DEPS) -->8 snip here 8<-- So the basic ideas are: - add a rule for constructing the .d files - let the build depend on the .d2 files >>> >>> IOW I wonder whether this really is any different from what we >>> do now (minus bugs/quirks in make itself, of course). And from this >>> as well as your original mail I still don't understand what's actually >>> broken with the current approach. >> >> The main problem is that the .d2 files used for determining which object >> files need to be (re-)built are based on the build before the last one. >> I'm not sure this is always the case, but at least when starting with a >> clean tree I need two invocations of "make" to get all .d2 files built. > > But that's correct: They're not needed _until_ a rebuild happens. > And by way of make's rebuilding of makefiles (if there are suitable > rules) they should appear _before_ any .o gets rebuilt, and even > before make evaluates which ones need rebuilding. Okay. Then the issue I'm after seems to be a different one. And now I know what is wrong with the build in tools/tests/depriv: There are no .o files specified for depriv-fd-checker, so it is built via: gcc $(CFLAGS) depriv-fd-checker.c $(LDFLAGS) -o depriv-fd-checker I'm suspecting the source file is specified via $<, as the resulting command line after touching one of the headers is: gcc $(CFLAGS) depriv-fd-checker.c $(LDFLAGS) -o depriv-fd-checker So all files the program is depending on are added into the parameters of gcc. This is the reason for the build error I'm seeing. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 3/6] xen/common: Use %*pb[l] instead of {cpu, node}mask_scn{, list}printf()
This removes all use of keyhandler_scratch as a bounce-buffer for the rendered
string. In some cases, collapse combine adjacent printk()'s which are writing
parts of the same line.
No functional change.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
CC: Stefano Stabellini
CC: Julien Grall
CC: Juergen Gross
---
xen/common/cpupool.c | 12 +++-
xen/common/event_channel.c | 6 ++
xen/common/keyhandler.c| 35 +--
3 files changed, 14 insertions(+), 39 deletions(-)
diff --git a/xen/common/cpupool.c b/xen/common/cpupool.c
index 1e8edcb..16ca4c4 100644
--- a/xen/common/cpupool.c
+++ b/xen/common/cpupool.c
@@ -732,12 +732,6 @@ int cpupool_do_sysctl(struct xen_sysctl_cpupool_op *op)
return ret;
}
-static void print_cpumap(const char *str, const cpumask_t *map)
-{
-cpulist_scnprintf(keyhandler_scratch, sizeof(keyhandler_scratch), map);
-printk("%s: %s\n", str, keyhandler_scratch);
-}
-
void dump_runq(unsigned char key)
{
unsigned longflags;
@@ -751,17 +745,17 @@ void dump_runq(unsigned char key)
sched_smt_power_savings? "enabled":"disabled");
printk("NOW=%"PRI_stime"\n", now);
-print_cpumap("Online Cpus", &cpu_online_map);
+printk("Online Cpus: %*pbl\n", nr_cpu_ids, &cpu_online_map);
if ( !cpumask_empty(&cpupool_free_cpus) )
{
-print_cpumap("Free Cpus", &cpupool_free_cpus);
+printk("Free Cpus: %*pbl\n", nr_cpu_ids, &cpupool_free_cpus);
schedule_dump(NULL);
}
for_each_cpupool(c)
{
printk("Cpupool %d:\n", (*c)->cpupool_id);
-print_cpumap("Cpus", (*c)->cpu_valid);
+printk("Cpus: %*pbl\n", nr_cpu_ids, (*c)->cpu_valid);
schedule_dump(*c);
}
diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index 381f30e..f34d4f0 100644
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -1377,11 +1377,9 @@ static void domain_dump_evtchn_info(struct domain *d)
unsigned int port;
int irq;
-bitmap_scnlistprintf(keyhandler_scratch, sizeof(keyhandler_scratch),
- d->poll_mask, d->max_vcpus);
printk("Event channel information for domain %d:\n"
- "Polling vCPUs: {%s}\n"
- "port [p/m/s]\n", d->domain_id, keyhandler_scratch);
+ "Polling vCPUs: {%*pbl}\n"
+ "port [p/m/s]\n", d->domain_id, d->max_vcpus, d->poll_mask);
spin_lock(&d->event_lock);
diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c
index 777c8e9..93ae738 100644
--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -250,22 +250,6 @@ static void reboot_machine(unsigned char key, struct
cpu_user_regs *regs)
machine_restart(0);
}
-static void cpuset_print(char *set, int size, const cpumask_t *mask)
-{
-*set++ = '{';
-set += cpulist_scnprintf(set, size-2, mask);
-*set++ = '}';
-*set++ = '\0';
-}
-
-static void nodeset_print(char *set, int size, const nodemask_t *mask)
-{
-*set++ = '[';
-set += nodelist_scnprintf(set, size-2, mask);
-*set++ = ']';
-*set++ = '\0';
-}
-
static void periodic_timer_print(char *str, int size, uint64_t period)
{
if ( period == 0 )
@@ -298,14 +282,14 @@ static void dump_domains(unsigned char key)
process_pending_softirqs();
printk("General information for domain %u:\n", d->domain_id);
-cpuset_print(tmpstr, sizeof(tmpstr), d->dirty_cpumask);
printk("refcnt=%d dying=%d pause_count=%d\n",
atomic_read(&d->refcnt), d->is_dying,
atomic_read(&d->pause_count));
printk("nr_pages=%d xenheap_pages=%d shared_pages=%u
paged_pages=%u "
- "dirty_cpus=%s max_pages=%u\n", d->tot_pages, d->xenheap_pages,
-atomic_read(&d->shr_pages), atomic_read(&d->paged_pages),
-tmpstr, d->max_pages);
+ "dirty_cpus={%*pbl} max_pages=%u\n",
+ d->tot_pages, d->xenheap_pages, atomic_read(&d->shr_pages),
+ atomic_read(&d->paged_pages), nr_cpu_ids, d->dirty_cpumask,
+ d->max_pages);
printk("handle=%02x%02x%02x%02x-%02x%02x-%02x%02x-"
"%02x%02x-%02x%02x%02x%02x%02x%02x vm_assist=%08lx\n",
d->handle[ 0], d->handle[ 1], d->handle[ 2], d->handle[ 3],
@@ -324,8 +308,8 @@ static void dump_domains(unsigned char key)
dump_pageframe_info(d);
-nodeset_print(tmpstr, sizeof(tmpstr), &d->node_affinity);
-printk("NODE affinity for domain %d: %s\n", d->domain_id, tmpstr);
+printk("NODE affinity for domain %d: [%*pbl]\n",
+ d->domain_id, MAX_NUMNODES, &d->node_affinity);
printk("VCPU information and callbacks for domain %u:\n",
d->domain_id);
@@ -343,10 +327,9 @@ static void dump_domains(unsigned char key)
if ( vcpu_cpu_dirty(v) )
printk("
[Xen-devel] [PATCH 4/6] xen/x86: Use %*pb[l] instead of cpumask_scn{, list}printf()
This removes all use of keyhandler_scratch as a bounce-buffer for the rendered
string.
No functional change.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
---
xen/arch/x86/cpu/mcheck/mce.c | 9 ++---
xen/arch/x86/crash.c | 7 ++-
xen/arch/x86/irq.c| 7 ++-
3 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index 32273d9..a860693 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -535,9 +535,12 @@ void mcheck_cmn_handler(const struct cpu_user_regs *regs)
mc_panic("MCE: No CPU found valid MCE, need reset");
if ( !cpumask_empty(&mce_fatal_cpus) )
{
-char *ebufp, ebuf[96] = "MCE: Fatal error happened on CPUs ";
-ebufp = ebuf + strlen(ebuf);
-cpumask_scnprintf(ebufp, 95 - strlen(ebuf), &mce_fatal_cpus);
+char ebuf[96];
+
+snprintf(ebuf, sizeof(ebuf),
+ "MCE: Fatal error happened on CPUs %*pd",
+ nr_cpu_ids, &mce_fatal_cpus);
+
mc_panic(ebuf);
}
atomic_set(&found_error, 0);
diff --git a/xen/arch/x86/crash.c b/xen/arch/x86/crash.c
index 8d74258..d4b83ad 100644
--- a/xen/arch/x86/crash.c
+++ b/xen/arch/x86/crash.c
@@ -159,11 +159,8 @@ static void nmi_shootdown_cpus(void)
if ( cpumask_empty(&waiting_to_crash) )
printk("Shot down all CPUs\n");
else
-{
-cpulist_scnprintf(keyhandler_scratch, sizeof keyhandler_scratch,
- &waiting_to_crash);
-printk("Failed to shoot down CPUs {%s}\n", keyhandler_scratch);
-}
+printk("Failed to shoot down CPUs {%*pbl}\n",
+ nr_cpu_ids, &waiting_to_crash);
/* Crash shutdown any IOMMU functionality as the crashdump kernel is not
* happy when booting if interrupt/dma remapping is still enabled */
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index ec93ab6..ca51320 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -2302,11 +2302,8 @@ static void dump_irqs(unsigned char key)
spin_lock_irqsave(&desc->lock, flags);
-cpumask_scnprintf(keyhandler_scratch, sizeof(keyhandler_scratch),
- desc->affinity);
-printk(" IRQ:%4d affinity:%s vec:%02x type=%-15s"
- " status=%08x ",
- irq, keyhandler_scratch, desc->arch.vector,
+printk(" IRQ:%4d affinity:%*pb vec:%02x type=%-15s status=%08x ",
+ irq, nr_cpu_ids, desc->affinity, desc->arch.vector,
desc->handler->typename, desc->status);
if ( ssid )
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 1/6] xen/vsprintf: Introduce %*pb[l] for printing bitmaps
The format identifier is consistent with Linux. The code is adapted from
bitmap_scn{,list}printf() but cleaned up.
This change allows all callers to avoid needing a secondary buffer to render a
cpumask/nodemask into.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
CC: Stefano Stabellini
CC: Julien Grall
CC: George Dunlap
CC: Dario Faggioli
---
docs/misc/printk-formats.txt | 8
xen/common/vsprintf.c| 97
2 files changed, 105 insertions(+)
diff --git a/docs/misc/printk-formats.txt b/docs/misc/printk-formats.txt
index 525108f..d0e1537 100644
--- a/docs/misc/printk-formats.txt
+++ b/docs/misc/printk-formats.txt
@@ -13,6 +13,14 @@ Raw buffer as hex string:
Up to 64 characters. Buffer length expected via the field_width
paramter. i.e. printk("%*ph", 8, buffer);
+Bitmaps (e.g. cpumask/nodemask):
+
+ %*pb4321
+ %*pbl 0,5,8-9,14
+
+ Print a bitmap as either a hex string, or a range list. Bitmap length
+ (in bits) expected via the field_width parameter.
+
Symbol/Function pointers:
%ps Symbol name with condition offset and size (iff offset != 0)
diff --git a/xen/common/vsprintf.c b/xen/common/vsprintf.c
index f92fb67..1750e5e 100644
--- a/xen/common/vsprintf.c
+++ b/xen/common/vsprintf.c
@@ -264,6 +264,88 @@ static char *string(char *str, char *end, const char *s,
return str;
}
+/* Print a bitmap as '0-3,6-15' */
+static char *print_bitmap_list(char *str, char *end,
+ const unsigned long *bitmap, int nr_bits)
+{
+/* current bit is 'cur', most recently seen range is [rbot, rtop] */
+int cur, rbot, rtop;
+bool first = true;
+
+rbot = cur = find_first_bit(bitmap, nr_bits);
+while ( cur < nr_bits )
+{
+rtop = cur;
+cur = find_next_bit(bitmap, nr_bits, cur + 1);
+
+if ( cur < nr_bits && cur <= rtop + 1 )
+continue;
+
+if ( !first )
+{
+if ( str < end )
+*str = ',';
+str++;
+}
+first = false;
+
+str = number(str, end, rbot, 10, -1, -1, 0);
+if ( rbot < rtop )
+{
+if ( str < end )
+*str = '-';
+str++;
+
+str = number(str, end, rtop, 10, -1, -1, 0);
+}
+
+rbot = cur;
+}
+
+return str;
+}
+
+/* Print a bitmap as a comma separated hex string. */
+static char *print_bitmap_string(char *str, char *end,
+ const unsigned long *bitmap, int nr_bits)
+{
+const unsigned int CHUNKSZ = 32;
+unsigned int chunksz;
+int i;
+bool first = true;
+
+chunksz = nr_bits & (CHUNKSZ - 1);
+if ( chunksz == 0 )
+chunksz = CHUNKSZ;
+
+/*
+ * First iteration copes with the trailing partial word if nr_bits isn't a
+ * round multiple of CHUNKSZ. All subsequent iterations work on a
+ * complete CHUNKSZ block.
+ */
+for ( i = ROUNDUP(nr_bits, CHUNKSZ) - CHUNKSZ; i >= 0; i -= CHUNKSZ )
+{
+unsigned int chunkmask = (1ull << chunksz) - 1;
+unsigned int word = i / BITS_PER_LONG;
+unsigned int offset = i % BITS_PER_LONG;
+unsigned long val = (bitmap[word] >> offset) & chunkmask;
+
+if ( !first )
+{
+if ( str < end )
+*str = ',';
+str++;
+}
+first = false;
+
+str = number(str, end, val, 16, DIV_ROUND_UP(chunksz, 4), -1, ZEROPAD);
+
+chunksz = CHUNKSZ;
+}
+
+return str;
+}
+
static char *pointer(char *str, char *end, const char **fmt_ptr,
const void *arg, int field_width, int precision,
int flags)
@@ -273,6 +355,21 @@ static char *pointer(char *str, char *end, const char
**fmt_ptr,
/* Custom %p suffixes. See XEN_ROOT/docs/misc/printk-formats.txt */
switch ( fmt[1] )
{
+case 'b': /* Bitmap as hex, or list */
+++*fmt_ptr;
+
+if ( field_width < 0 )
+return str;
+
+if ( fmt[2] == 'l' )
+{
+++*fmt_ptr;
+
+return print_bitmap_list(str, end, arg, field_width);
+}
+
+return print_bitmap_string(str, end, arg, field_width);
+
case 'h': /* Raw buffer as hex string. */
{
const uint8_t *hex_buffer = arg;
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH RFC 6/6] xen/keyhandler: Drop keyhandler_scratch
With almost all users of keyhandler_scratch gone, clean up the 3 remaining
users and drop the buffer.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
The NUMA and periodic timer adjustments are definitely improvements. However,
this is RFC because of the EFI change, which might perhaps not wan to be done
this way. Suggestions welcome.
---
xen/arch/x86/numa.c | 11 ---
xen/common/efi/boot.c| 5 ++---
xen/common/keyhandler.c | 26 +++---
xen/include/xen/keyhandler.h | 3 ---
4 files changed, 13 insertions(+), 32 deletions(-)
diff --git a/xen/arch/x86/numa.c b/xen/arch/x86/numa.c
index a87987d..de13aca 100644
--- a/xen/arch/x86/numa.c
+++ b/xen/arch/x86/numa.c
@@ -372,7 +372,6 @@ static void dump_numa(unsigned char key)
{
s_time_t now = NOW();
unsigned int i, j, n;
-int err;
struct domain *d;
struct page_info *page;
unsigned int page_num_node[MAX_NUMNODES];
@@ -454,12 +453,10 @@ static void dump_numa(unsigned char key)
{
unsigned int start_cpu = ~0U;
-err = snprintf(keyhandler_scratch, 12, "%3u",
-vnuma->vnode_to_pnode[i]);
-if ( err < 0 || vnuma->vnode_to_pnode[i] == NUMA_NO_NODE )
-strlcpy(keyhandler_scratch, "???", sizeof(keyhandler_scratch));
-
-printk(" %3u: pnode %s,", i, keyhandler_scratch);
+if ( vnuma->vnode_to_pnode[i] == NUMA_NO_NODE )
+printk(" %3u: pnode ???,", i);
+else
+printk(" %3u: pnode %3u,", i, vnuma->vnode_to_pnode[i]);
printk(" vcpus ");
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 2f49731..aa9a666 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -489,7 +489,7 @@ static EFI_FILE_HANDLE __init
get_parent_handle(EFI_LOADED_IMAGE *loaded_image,
static EFI_GUID __initdata fs_protocol = SIMPLE_FILE_SYSTEM_PROTOCOL;
EFI_FILE_HANDLE dir_handle;
EFI_DEVICE_PATH *dp;
-CHAR16 *pathend, *ptr;
+CHAR16 *pathend, *ptr, buffer[128];
EFI_STATUS ret;
do {
@@ -506,8 +506,7 @@ static EFI_FILE_HANDLE __init
get_parent_handle(EFI_LOADED_IMAGE *loaded_image,
if ( ret != EFI_SUCCESS )
PrintErrMesg(L"OpenVolume failure", ret);
-#define buffer ((CHAR16 *)keyhandler_scratch)
-#define BUFFERSIZE sizeof(keyhandler_scratch)
+#define BUFFERSIZE sizeof(buffer)
for ( dp = loaded_image->FilePath, *buffer = 0;
DevicePathType(dp) != END_DEVICE_PATH_TYPE;
dp = (void *)dp + DevicePathNodeLength(dp) )
diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c
index 93ae738..64e5783 100644
--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -29,8 +29,6 @@ static keyhandler_fn_t show_handlers, dump_hwdom_registers,
static irq_keyhandler_fn_t do_toggle_alt_key, dump_registers,
reboot_machine, run_all_keyhandlers, do_debug_key;
-char keyhandler_scratch[1024];
-
static struct keyhandler {
union {
keyhandler_fn_t *fn;
@@ -250,25 +248,11 @@ static void reboot_machine(unsigned char key, struct
cpu_user_regs *regs)
machine_restart(0);
}
-static void periodic_timer_print(char *str, int size, uint64_t period)
-{
-if ( period == 0 )
-{
-strlcpy(str, "No periodic timer", size);
-return;
-}
-
-snprintf(str, size,
- "%u Hz periodic timer (period %u ms)",
- 10/(int)period, (int)period/100);
-}
-
static void dump_domains(unsigned char key)
{
struct domain *d;
struct vcpu *v;
s_time_t now = NOW();
-#define tmpstr keyhandler_scratch
printk("'%c' pressed -> dumping domain info (now=0x%X:%08X)\n", key,
(u32)(now>>32), (u32)now);
@@ -333,8 +317,13 @@ static void dump_domains(unsigned char key)
printk("pause_count=%d pause_flags=%lx\n",
atomic_read(&v->pause_count), v->pause_flags);
arch_dump_vcpu_info(v);
-periodic_timer_print(tmpstr, sizeof(tmpstr), v->periodic_period);
-printk("%s\n", tmpstr);
+
+if ( v->periodic_period == 0 )
+printk("No periodic timer\n");
+else
+printk("%u Hz periodic timer (period %u ms)\n",
+ 10/(int)v->periodic_period,
+ (int)v->periodic_period/100);
}
}
@@ -355,7 +344,6 @@ static void dump_domains(unsigned char key)
arch_dump_shared_mem_info();
rcu_read_unlock(&domlist_read_lock);
-#undef tmpstr
}
static cpumask_t read_clocks_cpumask;
diff --git a/xen/include/xen/keyhandler.h b/xen/include/xen/keyhandler.h
index 06c05c8..5131e86 100644
--- a/xen/include/xen/keyhandler.h
+++ b/xen/include/xen/keyhandler.h
@@ -48,7 +48,4 @@ void register_irq_keyhandler(unsigned char key,
/* Inject a keypress into the key-handling subsystem. */
[Xen-devel] [PATCH 5/6] xen/bitmap: Drop all bitmap_scn{, list}printf() infrastructure
All callers have been convered to using %*pb[l]. In the unlikely case that
future code wants to retain this functionaly, it can be replicated in a more
convenient fashon with snprintf().
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
CC: Stefano Stabellini
CC: Julien Grall
CC: George Dunlap
---
xen/common/bitmap.c| 105 -
xen/include/xen/bitmap.h | 6 ---
xen/include/xen/cpumask.h | 18
xen/include/xen/nodemask.h | 34 ---
4 files changed, 163 deletions(-)
diff --git a/xen/common/bitmap.c b/xen/common/bitmap.c
index f498ee6..34de387 100644
--- a/xen/common/bitmap.c
+++ b/xen/common/bitmap.c
@@ -300,111 +300,6 @@ int __bitmap_weight(const unsigned long *bitmap, int bits)
#endif
EXPORT_SYMBOL(__bitmap_weight);
-/*
- * Bitmap printing & parsing functions: first version by Bill Irwin,
- * second version by Paul Jackson, third by Joe Korty.
- */
-
-#define CHUNKSZ32
-#define nbits_to_hold_value(val) fls(val)
-#define roundup_power2(val,modulus)(((val) + (modulus) - 1) & ~((modulus)
- 1))
-#define unhex(c) (isdigit(c) ? (c - '0') : (toupper(c) -
'A' + 10))
-#define BASEDEC 10 /* fancier cpuset lists input in decimal */
-
-/**
- * bitmap_scnprintf - convert bitmap to an ASCII hex string.
- * @buf: byte buffer into which string is placed
- * @buflen: reserved size of @buf, in bytes
- * @maskp: pointer to bitmap to convert
- * @nmaskbits: size of bitmap, in bits
- *
- * Exactly @nmaskbits bits are displayed. Hex digits are grouped into
- * comma-separated sets of eight digits per set.
- */
-int bitmap_scnprintf(char *buf, unsigned int buflen,
- const unsigned long *maskp, int nmaskbits)
-{
- int i, word, bit, len = 0;
- unsigned long val;
- const char *sep = "";
- int chunksz;
- u32 chunkmask;
-
- chunksz = nmaskbits & (CHUNKSZ - 1);
- if (chunksz == 0)
- chunksz = CHUNKSZ;
-
- i = roundup_power2(nmaskbits, CHUNKSZ) - CHUNKSZ;
- for (; i >= 0; i -= CHUNKSZ) {
- chunkmask = ((1ULL << chunksz) - 1);
- word = i / BITS_PER_LONG;
- bit = i % BITS_PER_LONG;
- val = (maskp[word] >> bit) & chunkmask;
- len += scnprintf(buf+len, buflen-len, "%s%0*lx", sep,
- (chunksz+3)/4, val);
- chunksz = CHUNKSZ;
- sep = ",";
- }
- return len;
-}
-EXPORT_SYMBOL(bitmap_scnprintf);
-
-/*
- * bscnl_emit(buf, buflen, rbot, rtop, bp)
- *
- * Helper routine for bitmap_scnlistprintf(). Write decimal number
- * or range to buf, suppressing output past buf+buflen, with optional
- * comma-prefix. Return len of what would be written to buf, if it
- * all fit.
- */
-static inline int bscnl_emit(char *buf, int buflen, int rbot, int rtop, int
len)
-{
- if (len > 0)
- len += scnprintf(buf + len, buflen - len, ",");
- if (rbot == rtop)
- len += scnprintf(buf + len, buflen - len, "%d", rbot);
- else
- len += scnprintf(buf + len, buflen - len, "%d-%d", rbot, rtop);
- return len;
-}
-
-/**
- * bitmap_scnlistprintf - convert bitmap to list format ASCII string
- * @buf: byte buffer into which string is placed
- * @buflen: reserved size of @buf, in bytes
- * @maskp: pointer to bitmap to convert
- * @nmaskbits: size of bitmap, in bits
- *
- * Output format is a comma-separated list of decimal numbers and
- * ranges. Consecutively set bits are shown as two hyphen-separated
- * decimal numbers, the smallest and largest bit numbers set in
- * the range. Output format is compatible with the format
- * accepted as input by bitmap_parselist().
- *
- * The return value is the number of characters which were output,
- * excluding the trailing '\0'.
- */
-int bitmap_scnlistprintf(char *buf, unsigned int buflen,
- const unsigned long *maskp, int nmaskbits)
-{
- int len = 0;
- /* current bit is 'cur', most recently seen range is [rbot, rtop] */
- int cur, rbot, rtop;
-
- rbot = cur = find_first_bit(maskp, nmaskbits);
- while (cur < nmaskbits) {
- rtop = cur;
- cur = find_next_bit(maskp, nmaskbits, cur+1);
- if (cur >= nmaskbits || cur > rtop + 1) {
- len = bscnl_emit(buf, buflen, rbot, rtop, len);
- rbot = cur;
- }
- }
- if (!len && buflen)
- *buf = 0;
- return len;
-}
-EXPORT_SYMBOL(bitmap_scnlistprintf);
/**
* bitmap_find_free_region - find a contiguous aligned mem region
diff --git a/xen/include/xen/bitmap.h b/xen/include/xen/bitmap.h
index e2a3686..fe3c720 100644
--- a/xen/include/xen/bitmap.h
+++ b/xen/include/xen/bitmap.h
@@ -40,8 +40,6 @@
* bitmap_weight(src, nbits) Hamming Weight: number set bi
[Xen-devel] [PATCH 0/6] xen: Use %*pb[l] for printing bitmaps
I was trying to debug a cpumask problem, and got irritated with how awkard it
was to render and print the masks. Luckily, the fix is quite simple and far
nicer to use.
The overall diffstat is to patch 5 is:
add/remove: 0/4 grow/shrink: 2/11 up/down: 603/-1191 (-588)
Function old new delta
vsnprintf 27753348+573
dump_runq279 309 +30
null_dump675 659 -16
dump_irqs718 702 -16
machine_crash_shutdown 594 570 -24
csched_dump 527 503 -24
rt_dump_vcpu.isra254 222 -32
dump_evtchn_info 657 625 -32
print_cpumap 55 - -55
cpuset_print.constprop61 - -61
csched_dump_pcpu 473 401 -72
mcheck_cmn_handler 12241146 -78
null_dump_pcpu 493 413 -80
dump_domains11601064 -96
csched2_dump15491389-160
bitmap_scnprintf 193 --193
bitmap_scnlistprintf 252 --252
Total: Before=3295448, After=3294860, chg -0.02%
Andrew Cooper (6):
xen/vsprintf: Introduce %*pb[l] for printing bitmaps
xen/sched: Use %*pb[l] instead of cpumask_scn{,list}printf()
xen/common: Use %*pb[l] instead of {cpu,node}mask_scn{,list}printf()
xen/x86: Use %*pb[l] instead of cpumask_scn{,list}printf()
xen/bitmap: Drop all bitmap_scn{,list}printf() infrastructure
xen/keyhandler: Drop keyhandler_scratch
docs/misc/printk-formats.txt | 8
xen/arch/x86/cpu/mcheck/mce.c | 9 ++--
xen/arch/x86/crash.c | 7 +--
xen/arch/x86/irq.c| 7 +--
xen/arch/x86/numa.c | 11 ++---
xen/common/bitmap.c | 105 --
xen/common/cpupool.c | 12 ++---
xen/common/efi/boot.c | 5 +-
xen/common/event_channel.c| 6 +--
xen/common/keyhandler.c | 61 +++-
xen/common/sched_credit.c | 17 ++-
xen/common/sched_credit2.c| 26 ---
xen/common/sched_null.c | 15 ++
xen/common/sched_rt.c | 5 +-
xen/common/vsprintf.c | 97 ++
xen/include/xen/bitmap.h | 6 ---
xen/include/xen/cpumask.h | 18
xen/include/xen/keyhandler.h | 3 --
xen/include/xen/nodemask.h| 34 --
19 files changed, 163 insertions(+), 289 deletions(-)
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 2/6] xen/sched: Use %*pb[l] instead of cpumask_scn{, list}printf()
This removes all use of keyhandler_scratch as a bounce-buffer for the rendered
string. In some cases, collapse combine adjacent printk()'s which are writing
parts of the same line.
No functional change.
Signed-off-by: Andrew Cooper
---
CC: George Dunlap
CC: Dario Faggioli
CC: Josh Whitehead
CC: Robert VanVossen
CC: Meng Xu
---
xen/common/sched_credit.c | 17 +
xen/common/sched_credit2.c | 26 +-
xen/common/sched_null.c| 15 +--
xen/common/sched_rt.c | 5 ++---
4 files changed, 21 insertions(+), 42 deletions(-)
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 84e744b..fe48e1b 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -2044,7 +2044,6 @@ csched_dump_pcpu(const struct scheduler *ops, int cpu)
spinlock_t *lock;
unsigned long flags;
int loop;
-#define cpustr keyhandler_scratch
/*
* We need both locks:
@@ -2059,11 +2058,10 @@ csched_dump_pcpu(const struct scheduler *ops, int cpu)
spc = CSCHED_PCPU(cpu);
runq = &spc->runq;
-cpumask_scnprintf(cpustr, sizeof(cpustr), per_cpu(cpu_sibling_mask, cpu));
-printk("CPU[%02d] nr_run=%d, sort=%d, sibling=%s, ",
- cpu, spc->nr_runnable, spc->runq_sort_last, cpustr);
-cpumask_scnprintf(cpustr, sizeof(cpustr), per_cpu(cpu_core_mask, cpu));
-printk("core=%s\n", cpustr);
+printk("CPU[%02d] nr_run=%d, sort=%d, sibling=%*pb, core=%*pb\n",
+ cpu, spc->nr_runnable, spc->runq_sort_last,
+ nr_cpu_ids, per_cpu(cpu_sibling_mask, cpu),
+ nr_cpu_ids, per_cpu(cpu_core_mask, cpu));
/* current VCPU (nothing to say if that's the idle vcpu). */
svc = CSCHED_VCPU(curr_on_cpu(cpu));
@@ -2086,7 +2084,6 @@ csched_dump_pcpu(const struct scheduler *ops, int cpu)
pcpu_schedule_unlock(lock, cpu);
spin_unlock_irqrestore(&prv->lock, flags);
-#undef cpustr
}
static void
@@ -2099,8 +2096,6 @@ csched_dump(const struct scheduler *ops)
spin_lock_irqsave(&prv->lock, flags);
-#define idlers_buf keyhandler_scratch
-
printk("info:\n"
"\tncpus = %u\n"
"\tmaster = %u\n"
@@ -2127,8 +2122,7 @@ csched_dump(const struct scheduler *ops)
prv->ticks_per_tslice,
prv->vcpu_migr_delay/ MICROSECS(1));
-cpumask_scnprintf(idlers_buf, sizeof(idlers_buf), prv->idlers);
-printk("idlers: %s\n", idlers_buf);
+printk("idlers: %*pb\n", nr_cpu_ids, prv->idlers);
printk("active vcpus:\n");
loop = 0;
@@ -2151,7 +2145,6 @@ csched_dump(const struct scheduler *ops)
vcpu_schedule_unlock(lock, svc->vcpu);
}
}
-#undef idlers_buf
spin_unlock_irqrestore(&prv->lock, flags);
}
diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 7438481..1df9d5f 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -3650,12 +3650,11 @@ dump_pcpu(const struct scheduler *ops, int cpu)
{
struct csched2_private *prv = csched2_priv(ops);
struct csched2_vcpu *svc;
-#define cpustr keyhandler_scratch
-cpumask_scnprintf(cpustr, sizeof(cpustr), per_cpu(cpu_sibling_mask, cpu));
-printk("CPU[%02d] runq=%d, sibling=%s, ", cpu, c2r(cpu), cpustr);
-cpumask_scnprintf(cpustr, sizeof(cpustr), per_cpu(cpu_core_mask, cpu));
-printk("core=%s\n", cpustr);
+printk("CPU[%02d] runq=%d, sibling=%*pb, core=%*pb\n",
+ cpu, c2r(cpu),
+ nr_cpu_ids, per_cpu(cpu_sibling_mask, cpu),
+ nr_cpu_ids, per_cpu(cpu_core_mask, cpu));
/* current VCPU (nothing to say if that's the idle vcpu) */
svc = csched2_vcpu(curr_on_cpu(cpu));
@@ -3664,7 +3663,6 @@ dump_pcpu(const struct scheduler *ops, int cpu)
printk("\trun: ");
csched2_dump_vcpu(prv, svc);
}
-#undef cpustr
}
static void
@@ -3674,7 +3672,6 @@ csched2_dump(const struct scheduler *ops)
struct csched2_private *prv = csched2_priv(ops);
unsigned long flags;
unsigned int i, j, loop;
-#define cpustr keyhandler_scratch
/*
* We need the private scheduler lock as we access global
@@ -3692,29 +3689,25 @@ csched2_dump(const struct scheduler *ops)
fraction = (prv->rqd[i].avgload * 100) >> prv->load_precision_shift;
-cpulist_scnprintf(cpustr, sizeof(cpustr), &prv->rqd[i].active);
printk("Runqueue %d:\n"
"\tncpus = %u\n"
- "\tcpus = %s\n"
+ "\tcpus = %*pbl\n"
"\tmax_weight = %u\n"
"\tpick_bias = %u\n"
"\tinstload = %d\n"
"\taveload= %"PRI_stime" (~%"PRI_stime"%%)\n",
i,
cpumask_weight(&prv->rqd[i].active),
- cpustr,
+ nr_cpu_ids, &prv->rqd[i].active,
prv->rqd[i].max_weight,
prv->rqd[i].pick_
[Xen-devel] [PATCH] tools: specifically enable VirtFS in Linux QEMU builds
9pfs support has been a documented feature since Xen 4.9, but QEMU will
not be built with backend support unless libcap and libattr dev packages
are installed.
This patch modifies the README to call out those packages as pre-requisites
for Linux builds and specifically enables VirtFS in the configure line
for QEMU so that an error message is displayed if they are missing.
Signed-off-by: Paul Durrant
---
Cc: Andrew Cooper
Cc: George Dunlap
Cc: Ian Jackson
Cc: Jan Beulich
Cc: Julien Grall
Cc: Konrad Rzeszutek Wilk
Cc: Stefano Stabellini
Cc: Tim Deegan
Cc: Wei Liu
---
README | 2 ++
tools/Makefile | 11 +++
2 files changed, 13 insertions(+)
diff --git a/README b/README
index 4b95b21c7b..1a4e4b2c1b 100644
--- a/README
+++ b/README
@@ -56,6 +56,8 @@ provided by your OS distributor:
greater.
* Development install of GLib v2.0 (e.g. libglib2.0-dev)
* Development install of Pixman (e.g. libpixman-1-dev)
+* Development install of libcap (e.g. libcap-dev) [Linux only]
+* Development install of libattr (e.g. libattr1-dev) [Linux only]
* pkg-config
* bridge-utils package (/sbin/brctl)
* iproute package (/sbin/ip)
diff --git a/tools/Makefile b/tools/Makefile
index 67977ad850..e74efb8a6e 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -216,6 +216,11 @@ else
QEMU_XEN_ENABLE_DEBUG :=
endif
+#
+# 9pfs support is a documented feature but it depends on a QEMU with
+# VirtFS enabled. However VirtFS is a Linux-only option so only enable
+# it for Linux builds.
+#
subdir-all-qemu-xen-dir: qemu-xen-dir-find
unset MAKELEVEL; \
if test -d $(QEMU_UPSTREAM_LOC) ; then \
@@ -232,10 +237,16 @@ subdir-all-qemu-xen-dir: qemu-xen-dir-find
else \
enable_trace_backend='' ; \
fi ; \
+ if [ "$(CONFIG_Linux)" = "y" ]; then \
+ enable_virtfs='--enable-virtfs' ; \
+ else \
+ enable_virtfs='' ; \
+ fi ; \
PKG_CONFIG_PATH=$(XEN_ROOT)/tools/pkg-config$${PKG_CONFIG_PATH:+:$${PKG_CONFIG_PATH}}
\
$$source/configure --enable-xen --target-list=i386-softmmu \
$(QEMU_XEN_ENABLE_DEBUG) \
$$enable_trace_backend \
+ $$enable_virtfs \
--prefix=$(LIBEXEC) \
--libdir=$(LIBEXEC_LIB) \
--includedir=$(LIBEXEC_INC) \
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] xen/manage: don't complain about an empty value in control/sysrq node
When guest receives a sysrq request from the host it acknowledges it by
writing '\0' to control/sysrq xenstore node. This, however, make xenstore
watch fire again but xenbus_scanf() fails to parse empty value with "%c"
format string:
sysrq: SysRq : Emergency Sync
Emergency Sync complete
xen:manage: Error -34 reading sysrq code in control/sysrq
Ignore -ERANGE the same way we already ignore -ENOENT, empty value in
control/sysrq is totally legal.
Signed-off-by: Vitaly Kuznetsov
---
This is a follow-up to my Xen toolstack patch:
https://lists.xenproject.org/archives/html/xen-devel/2018-09/msg00266.html
without it we're seeing -EPERM on write and the issue I'm trying to address
here stays hidden.
---
drivers/xen/manage.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
index c93d8ef8df34..5bb01a62f214 100644
--- a/drivers/xen/manage.c
+++ b/drivers/xen/manage.c
@@ -280,9 +280,11 @@ static void sysrq_handler(struct xenbus_watch *watch,
const char *path,
/*
* The Xenstore watch fires directly after registering it and
* after a suspend/resume cycle. So ENOENT is no error but
-* might happen in those cases.
+* might happen in those cases. ERANGE is observed when we get
+* an empty value (''), this happens when we acknowledge the
+* request by writing '\0' below.
*/
- if (err != -ENOENT)
+ if (err != -ENOENT && err != -ERANGE)
pr_err("Error %d reading sysrq code in control/sysrq\n",
err);
xenbus_transaction_end(xbt, 1);
--
2.14.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 01/16] x86: change name of parameter for various invlpg functions
On 09/04/2018 05:15 PM, Wei Liu wrote:
> They all incorrectly named a parameter virtual address while it should
> have been linear address.
>
> Requested-by: Andrew Cooper
> Signed-off-by: Wei Liu
> Acked-by: Jan Beulich
> Reviewed-by: Kevin Tian
> Acked-by: Boris Ostrovsky
Acked-by: George Dunlap
> ---
> xen/arch/x86/hvm/svm/svm.c | 14 +++---
> xen/arch/x86/hvm/vmx/vmx.c | 12 ++--
> xen/arch/x86/mm.c | 10 +-
> xen/arch/x86/mm/hap/hap.c | 2 +-
> xen/arch/x86/mm/shadow/multi.c | 14 +++---
> xen/arch/x86/mm/shadow/none.c | 2 +-
> xen/include/asm-x86/hvm/hvm.h | 6 +++---
> xen/include/asm-x86/hvm/svm/asid.h | 4 ++--
> xen/include/asm-x86/hvm/svm/svm.h | 4 ++--
> xen/include/asm-x86/paging.h | 3 ++-
> 10 files changed, 36 insertions(+), 35 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index 0b06e2ff11..34d55b4938 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -2488,18 +2488,18 @@ static void svm_vmexit_do_invalidate_cache(struct
> cpu_user_regs *regs)
> }
>
> static void svm_invlpga_intercept(
> -struct vcpu *v, unsigned long vaddr, uint32_t asid)
> +struct vcpu *v, unsigned long linear, uint32_t asid)
> {
> -svm_invlpga(vaddr,
> +svm_invlpga(linear,
> (asid == 0)
> ? v->arch.hvm.n1asid.asid
> : vcpu_nestedhvm(v).nv_n2asid.asid);
> }
>
> -static void svm_invlpg_intercept(unsigned long vaddr)
> +static void svm_invlpg_intercept(unsigned long linear)
> {
> -HVMTRACE_LONG_2D(INVLPG, 0, TRC_PAR_LONG(vaddr));
> -paging_invlpg(current, vaddr);
> +HVMTRACE_LONG_2D(INVLPG, 0, TRC_PAR_LONG(linear));
> +paging_invlpg(current, linear);
> }
>
> static bool is_invlpg(const struct x86_emulate_state *state,
> @@ -2512,9 +2512,9 @@ static bool is_invlpg(const struct x86_emulate_state
> *state,
> (ext & 7) == 7;
> }
>
> -static void svm_invlpg(struct vcpu *v, unsigned long vaddr)
> +static void svm_invlpg(struct vcpu *v, unsigned long linear)
> {
> -svm_asid_g_invlpg(v, vaddr);
> +svm_asid_g_invlpg(v, linear);
> }
>
> static bool svm_get_pending_event(struct vcpu *v, struct x86_event *info)
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index e926b0b28e..b2e1a28038 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -75,7 +75,7 @@ static void vmx_wbinvd_intercept(void);
> static void vmx_fpu_dirty_intercept(void);
> static int vmx_msr_read_intercept(unsigned int msr, uint64_t *msr_content);
> static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content);
> -static void vmx_invlpg(struct vcpu *v, unsigned long vaddr);
> +static void vmx_invlpg(struct vcpu *v, unsigned long linear);
>
> /* Values for domain's ->arch.hvm_domain.pi_ops.flags. */
> #define PI_CSW_FROM (1u << 0)
> @@ -2595,16 +2595,16 @@ static void vmx_dr_access(unsigned long
> exit_qualification,
> vmx_update_cpu_exec_control(v);
> }
>
> -static void vmx_invlpg_intercept(unsigned long vaddr)
> +static void vmx_invlpg_intercept(unsigned long linear)
> {
> -HVMTRACE_LONG_2D(INVLPG, /*invlpga=*/ 0, TRC_PAR_LONG(vaddr));
> -paging_invlpg(current, vaddr);
> +HVMTRACE_LONG_2D(INVLPG, /*invlpga=*/ 0, TRC_PAR_LONG(linear));
> +paging_invlpg(current, linear);
> }
>
> -static void vmx_invlpg(struct vcpu *v, unsigned long vaddr)
> +static void vmx_invlpg(struct vcpu *v, unsigned long linear)
> {
> if ( cpu_has_vmx_vpid )
> -vpid_sync_vcpu_gva(v, vaddr);
> +vpid_sync_vcpu_gva(v, linear);
> }
>
> static int vmx_vmfunc_intercept(struct cpu_user_regs *regs)
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index 84979f28d5..409814ce0a 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -5793,19 +5793,19 @@ const unsigned long *__init
> get_platform_badpages(unsigned int *array_size)
> return bad_pages;
> }
>
> -void paging_invlpg(struct vcpu *v, unsigned long va)
> +void paging_invlpg(struct vcpu *v, unsigned long linear)
> {
> -if ( !is_canonical_address(va) )
> +if ( !is_canonical_address(linear) )
> return;
>
> if ( paging_mode_enabled(v->domain) &&
> - !paging_get_hostmode(v)->invlpg(v, va) )
> + !paging_get_hostmode(v)->invlpg(v, linear) )
> return;
>
> if ( is_pv_vcpu(v) )
> -flush_tlb_one_local(va);
> +flush_tlb_one_local(linear);
> else
> -hvm_invlpg(v, va);
> +hvm_invlpg(v, linear);
> }
>
> /* Build a 32bit PSE page table using 4MB pages. */
> diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
> index c53d76cf69..3d651b94c3 100644
> --- a/xen/arch/x86/mm/hap/hap.c
> +++ b/xen/arch/x86/mm/hap/hap.c
> @@ -650,7 +650,7 @@ static int hap_page_fault(struct vcpu *v, unsigned long
> va,
>
Re: [Xen-devel] [PATCH v2 11/13] libxc: add xc_dom_tee_enable(...) function
On Mon, Sep 03, 2018 at 07:54:35PM +0300, Volodymyr Babchuk wrote: > This function uses XEN_DOMCTL_tee_op domctl to enable TEE > support for a given domain. > > Signed-off-by: Volodymyr Babchuk Take into account Julien's comment to the previous patch, this patch should be dropped. Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 07/16] x86/p2m/pod: make it build with !CONFIG_HVM
On Tue, Sep 04, 2018 at 06:24:28PM +0100, Julien Grall wrote: > Hi Wei, > > On 09/04/2018 05:15 PM, Wei Liu wrote: > > Populate-on-demand is HVM only. > > > > Provide a bunch of stubs for common p2m code and guard one invocation > > of guest_physmap_mark_populate_on_demand with is_hvm_domain. > > > > Put relevant fields in p2m_domain and code which touches those fields > > under CONFIG_HVM. > > Arm does not have any POD support. Would it be worth to introduce a > CONFIG_HAS_POD to avoid dummy function on Arm? That sounds fine. This will have the effect of not compiling PoD on Arm at all, which is good. I will wait for more reviews before reworking this patch. Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/domctl: Drop vcpu_alloc_lock
On Wed, Sep 05, 2018 at 08:15:25PM +0100, Andrew Cooper wrote: > Since its introduction in c/s 8cbb5278e "x86/AMD: Add support for AMD's OSVW > feature in guests", the OSVW data has been corrected to be per-domain rather > than per-vcpu, and is initialised during XEN_DOMCTL_createdomain. > > Furthermore, because XENPF_microcode_update uses hypercall continuations to > move between CPUs, it drops the vcpu_alloc_lock mid update, meaning that it > didn't provided the interlock guarantee that the OSVW patch was looking for in > the first place. > > This interlock serves no purpose, so take the opportunity to drop it and > remove a global spinlock from the hypervisor. > > Signed-off-by: Andrew Cooper Reviewed-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 5/5] xen/ARM: Restrict access to most HVM_PARAM's
> -Original Message-
> From: Andrew Cooper
> Sent: 06 September 2018 11:40
> To: Julien Grall ; Paul Durrant
> ; Xen-devel
> Cc: Jan Beulich ; Wei Liu ; Roger
> Pau Monne ; Stefano Stabellini
>
> Subject: Re: [PATCH 5/5] xen/ARM: Restrict access to most HVM_PARAM's
>
> On 06/09/18 11:36, Julien Grall wrote:
> > Hi Paul,
> >
> > On 06/09/18 10:29, Paul Durrant wrote:
> >>> -Original Message-
> >>> From: Andrew Cooper [mailto:andrew.coop...@citrix.com]
> >>> Sent: 05 September 2018 19:12
> >>> To: Xen-devel
> >>> Cc: Andrew Cooper ; Jan Beulich
> >>> ; Wei Liu ; Roger Pau Monne
> >>> ; Paul Durrant ;
> Stefano
> >>> Stabellini ; Julien Grall
> >>>
> >>> Subject: [PATCH 5/5] xen/ARM: Restrict access to most HVM_PARAM's
> >>>
> >>> ARM currently has no restrictions on toolstack and guest access to
> >>> the entire
> >>> HVM_PARAM block. As the paging/monitor/sharing features aren't
> under
> >>> security
> >>> support, this doesn't need an XSA.
> >>>
> >>> The CALLBACK_IRQ and {STORE,CONSOLE}_{PFN,EVTCHN} details
> exposed
> >>> read-only to
> >>> the guest, while the *_RING_PFN details are restricted to only
> >>> toolstack
> >>> access. No other parameters are used.
> >>>
> >>> Signed-off-by: Andrew Cooper
> >>> ---
> >>> CC: Jan Beulich
> >>> CC: Wei Liu
> >>> CC: Roger Pau Monné
> >>> CC: Paul Durrant
> >>> CC: Stefano Stabellini
> >>> CC: Julien Grall
> >>>
> >>> This is only compile tested, and based on my reading of the source.
> >>> There
> >>> might be other PARAMS needing including.
> >>> ---
> >>> xen/arch/arm/hvm.c | 62
> >>> +++---
> >>> 1 file changed, 59 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
> >>> index 76b27c9..3581ba2 100644
> >>> --- a/xen/arch/arm/hvm.c
> >>> +++ b/xen/arch/arm/hvm.c
> >>> @@ -31,6 +31,57 @@
> >>>
> >>> #include
> >>>
> >>> +static int hvm_allow_set_param(const struct domain *d, unsigned int
> >>> param)
> >>> +{
> >>> + switch ( param )
> >>> + {
> >>> + /*
> >>> + * The following parameters are intended for toolstack
> >>> usage only.
> >>> + * They may not be set by the domain.
> >>> + */
> >>> + case HVM_PARAM_CALLBACK_IRQ:
> >>> + case HVM_PARAM_STORE_PFN:
> >>> + case HVM_PARAM_STORE_EVTCHN:
> >>> + case HVM_PARAM_CONSOLE_PFN:
> >>> + case HVM_PARAM_CONSOLE_EVTCHN:
> >>
> >> Probably should remove the EVTCHN params from this list after fixing
> >> patch #3.
> >>
> >>> + case HVM_PARAM_PAGING_RING_PFN:
> >>> + case HVM_PARAM_MONITOR_RING_PFN:
> >>> + case HVM_PARAM_SHARING_RING_PFN:
> >>> + return d == current->domain ? -EPERM : 0;
> >>> +
> >>> + /* Writeable only by Xen, hole, deprecated, or
> >>> out-of-range. */
> >>> + default:
> >>> + return -EINVAL;
> >>> + }
> >>> +}
> >>> +
> >>> +static int hvm_allow_get_param(const struct domain *d, unsigned int
> >>> param)
> >>> +{
> >>> + switch ( param )
> >>> + {
> >>> + /* The following parameters can be read by the guest and
> >>> toolstack. */
> >>> + case HVM_PARAM_CALLBACK_IRQ:
> >>> + case HVM_PARAM_STORE_PFN:
> >>> + case HVM_PARAM_STORE_EVTCHN:
> >>> + case HVM_PARAM_CONSOLE_PFN:
> >>> + case HVM_PARAM_CONSOLE_EVTCHN:
> >>> + return 0;
> >>> +
> >>> + /*
> >>> + * The following parameters are intended for toolstack
> >>> usage only.
> >>> + * They may not be read by the domain.
> >>> + */
> >>> + case HVM_PARAM_PAGING_RING_PFN:
> >>> + case HVM_PARAM_MONITOR_RING_PFN:
> >>> + case HVM_PARAM_SHARING_RING_PFN:
> >>> + return d == current->domain ? -EPERM : 0;
> >>> +
> >>> + /* Hole, deprecated, or out-of-range. */
> >>> + default:
> >>> + return -EINVAL;
> >>> + }
> >>> +}
> >>> +
> >>> long do_hvm_op(unsigned long op,
> XEN_GUEST_HANDLE_PARAM(void)
> >>> arg)
> >>> {
> >>> long rc = 0;
> >>> @@ -46,9 +97,6 @@ long do_hvm_op(unsigned long op,
> >>> XEN_GUEST_HANDLE_PARAM(void) arg)
> >>> if ( copy_from_guest(&a, arg, 1) )
> >>> return -EFAULT;
> >>>
> >>> - if ( a.index >= HVM_NR_PARAMS )
> >>> - return -EINVAL;
> >>> -
> >>
> >> ASSERT here.
> >
> > I don't think this would be correct. This is an input from the guest,
> > so if you do fuzzing you will end up to get an hypervisor crash rather
> > than returning an error.
> >
> > A potential place for an ASSERT would be just before accessing
> > hvm.params. But then, technically the index should have been sanitized
> > by hvm_allow_{get,set}_param.
>
> Yeah - across all of these ASSERT() requests - using an assert for a
> boundary check doesn't do anything in the case where it matters most,
> and in this case, Julien is correct that it is a fully guest-controlled
> number at this point.
>
I'm just uneasy about removing a bounds check. If a logic error creeps into an
'allow' function i
