Re: [Xen-devel] [PATCH 01/12] arm:Kconfig Rename menu text
Hi Julien, On 03/13/2018 05:45 PM, Julien Grall wrote: Hi, On 12/03/18 12:42, mja...@caviumnetworks.com wrote: From: Manish Jaggi Rename the menu text to Errata Workarounds. Subsequent patches will add config options for SoC specific erratas. Well, your SoC is an Arm SoC, right? So what is the benefits of this new name? M It was based on your last comment "I would much prefer to see the memu "ARM errata workaround via..." renamed to "Errata Workarounds". So we have only one menu with all workarounds." ore that it still depends on HAS_ALTERNATIVE. check_workaroundXXX depends on it. So I kept it as is. If you think this patch is not required, I can drop it. Cheers, Signed-off-by: Manish Jaggi --- xen/arch/arm/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig index f58019d6ed..10a6d1a956 100644 --- a/xen/arch/arm/Kconfig +++ b/xen/arch/arm/Kconfig @@ -59,7 +59,7 @@ config SBSA_VUART_CONSOLE endmenu -menu "ARM errata workaround via the alternative framework" +menu "Errata workarounds" depends on HAS_ALTERNATIVE > config ARM64_ERRATUM_827319 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86: vlapic: Clear vector's TMR bit upon acceptance of edge-triggered interrupt to IRR
According to Intel SDM 10.8.4 Interrupt Acceptance for Fixed Interrupts: "The trigger mode register (TMR) indicates the trigger mode of the interrupt (see Figure 10-20). Upon acceptance of an interrupt into the IRR, the corresponding TMR bit is cleared for edge-triggered interrupts and set for level-triggered interrupts. If a TMR bit is set when an EOI cycle for its corresponding interrupt vector is generated, an EOI message is sent to all I/O APICs." Before this patch TMR-bit was cleared on LAPIC EOI which is not what real hardware does. This was also confirmed in KVM upstream commit a0c9a822bf37 ("KVM: dont clear TMR on EOI"). Behavior after this patch is aligned with both Intel SDM and KVM implementation. Signed-off-by: Liran Alon Reviewed-by: Boris Ostrovsky Signed-off-by: Boris Ostrovsky --- xen/arch/x86/hvm/vlapic.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c index 7387f91fe04e..9fb0066bfff7 100644 --- a/xen/arch/x86/hvm/vlapic.c +++ b/xen/arch/x86/hvm/vlapic.c @@ -161,6 +161,8 @@ void vlapic_set_irq(struct vlapic *vlapic, uint8_t vec, uint8_t trig) if ( trig ) vlapic_set_vector(vec, &vlapic->regs->data[APIC_TMR]); +else +vlapic_clear_vector(vec, &vlapic->regs->data[APIC_TMR]); if ( hvm_funcs.update_eoi_exit_bitmap ) hvm_funcs.update_eoi_exit_bitmap(target, vec, trig); @@ -461,7 +463,7 @@ void vlapic_handle_EOI(struct vlapic *vlapic, u8 vector) { struct domain *d = vlapic_domain(vlapic); -if ( vlapic_test_and_clear_vector(vector, &vlapic->regs->data[APIC_TMR]) ) +if ( vlapic_test_vector(vector, &vlapic->regs->data[APIC_TMR]) ) vioapic_update_EOI(d, vector); hvm_dpci_msi_eoi(d, vector); -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-3.18 test] 120665: regressions - FAIL
flight 120665 linux-3.18 real [real] http://logs.test-lab.xenproject.org/osstest/logs/120665/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 16 guest-localmigrate/x10 fail REGR. vs. 120276 Tests which are failing intermittently (not blocking): test-amd64-i386-qemut-rhel6hvm-intel 10 redhat-install fail in 120486 pass in 120665 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 16 guest-localmigrate/x10 fail in 120486 pass in 120665 test-amd64-amd64-xl-qemuu-debianhvm-amd64 16 guest-localmigrate/x10 fail in 120486 pass in 120665 test-amd64-i386-xl-qemut-win7-amd64 10 windows-install fail in 120486 pass in 120665 test-amd64-amd64-xl-qemut-win7-amd64 10 windows-install fail in 120486 pass in 120665 test-amd64-amd64-xl-qemuu-ws16-amd64 7 xen-boot fail pass in 120486 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-arm64-arm64-examine 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail in 120486 never pass test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail like 120276 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 120276 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 120276 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 120276 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 120276 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 120276 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 120276 test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-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-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass build-arm64-pvops 6 kernel-build fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 wi
[Xen-devel] [PATCH] xen: xenbus_dev_frontend: Really return response string
xenbus_command_reply() did not actually copy the response string and leaked stack content instead. Fixes: 9a6161fe73bd ("xen: return xenstore command failures via response instead of rc") Signed-off-by: Simon Gaiser --- PS: AFAICS this is not a security issue since /dev/xen/xenbus is normally only accessible by root and giving xenstore access to a less trusted entity probably has a bunch of other unintended consequences. drivers/xen/xenbus/xenbus_dev_frontend.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/xen/xenbus/xenbus_dev_frontend.c b/drivers/xen/xenbus/xenbus_dev_frontend.c index a493e99bed21..845a70fa7f79 100644 --- a/drivers/xen/xenbus/xenbus_dev_frontend.c +++ b/drivers/xen/xenbus/xenbus_dev_frontend.c @@ -403,7 +403,7 @@ static int xenbus_command_reply(struct xenbus_file_priv *u, { struct { struct xsd_sockmsg hdr; - const char body[16]; + char body[16]; } msg; int rc; @@ -412,6 +412,7 @@ static int xenbus_command_reply(struct xenbus_file_priv *u, msg.hdr.len = strlen(reply) + 1; if (msg.hdr.len > sizeof(msg.body)) return -E2BIG; + memcpy(&msg.body, reply, msg.hdr.len); mutex_lock(&u->reply_mutex); rc = queue_reply(&u->read_buffers, &msg, sizeof(msg.hdr) + msg.hdr.len); -- 2.16.2 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 3/3] xen: xenbus_dev_frontend: Verify body of XS_TRANSACTION_END
By guaranteeing that the argument of XS_TRANSACTION_END is valid we can assume that the transaction has been closed when we get an XS_ERROR response from xenstore (Note that we already verify that it's a valid transaction id). Signed-off-by: Simon Gaiser --- drivers/xen/xenbus/xenbus_dev_frontend.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/xen/xenbus/xenbus_dev_frontend.c b/drivers/xen/xenbus/xenbus_dev_frontend.c index 81a84b3c1c50..0d6d9264d6a9 100644 --- a/drivers/xen/xenbus/xenbus_dev_frontend.c +++ b/drivers/xen/xenbus/xenbus_dev_frontend.c @@ -429,6 +429,10 @@ static int xenbus_write_transaction(unsigned msg_type, { int rc; struct xenbus_transaction_holder *trans = NULL; + struct { + struct xsd_sockmsg hdr; + char body[]; + } *msg = (void *)u->u.buffer; if (msg_type == XS_TRANSACTION_START) { trans = kzalloc(sizeof(*trans), GFP_KERNEL); @@ -437,11 +441,15 @@ static int xenbus_write_transaction(unsigned msg_type, goto out; } list_add(&trans->list, &u->transactions); - } else if (u->u.msg.tx_id != 0 && - !xenbus_get_transaction(u, u->u.msg.tx_id)) + } else if (msg->hdr.tx_id != 0 && + !xenbus_get_transaction(u, msg->hdr.tx_id)) return xenbus_command_reply(u, XS_ERROR, "ENOENT"); + else if (msg_type == XS_TRANSACTION_END && +!(msg->hdr.len == 2 && + (!strcmp(msg->body, "T") || !strcmp(msg->body, "F" + return xenbus_command_reply(u, XS_ERROR, "EINVAL"); - rc = xenbus_dev_request_and_reply(&u->u.msg, u); + rc = xenbus_dev_request_and_reply(&msg->hdr, u); if (rc && trans) { list_del(&trans->list); kfree(trans); -- 2.16.2 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 1/3] xen: xenbus_dev_frontend: Fix XS_TRANSACTION_END handling
Commit fd8aa9095a95 ("xen: optimize xenbus driver for multiple concurrent xenstore accesses") made a subtle change to the semantic of xenbus_dev_request_and_reply() and xenbus_transaction_end(). Before on an error response to XS_TRANSACTION_END xenbus_dev_request_and_reply() would not decrement the active transaction counter. But xenbus_transaction_end() has always counted the transaction as finished regardless of the response. The new behavior is that xenbus_dev_request_and_reply() and xenbus_transaction_end() will always count the transaction as finished regardless the response code (handled in xs_request_exit()). But xenbus_dev_frontend tries to end a transaction on closing of the device if the XS_TRANSACTION_END failed before. Trying to close the transaction twice corrupts the reference count. So fix this by also considering a transaction closed if we have sent XS_TRANSACTION_END once regardless of the return code. Cc: # 4.11 Fixes: fd8aa9095a95 ("xen: optimize xenbus driver for multiple concurrent xenstore accesses") Signed-off-by: Simon Gaiser --- drivers/xen/xenbus/xenbus_dev_frontend.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/xen/xenbus/xenbus_dev_frontend.c b/drivers/xen/xenbus/xenbus_dev_frontend.c index a493e99bed21..81a84b3c1c50 100644 --- a/drivers/xen/xenbus/xenbus_dev_frontend.c +++ b/drivers/xen/xenbus/xenbus_dev_frontend.c @@ -365,7 +365,7 @@ void xenbus_dev_queue_reply(struct xb_req_data *req) if (WARN_ON(rc)) goto out; } - } else if (req->msg.type == XS_TRANSACTION_END) { + } else if (req->type == XS_TRANSACTION_END) { trans = xenbus_get_transaction(u, req->msg.tx_id); if (WARN_ON(!trans)) goto out; -- 2.16.2 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 2/3] xen: xenbus: Catch closing of non existent transactions
Users of the xenbus functions should never close a non existent transaction (for example by trying to closing the same transaction twice) but better catch it in xs_request_exit() than to corrupt the reference counter. Signed-off-by: Simon Gaiser --- drivers/xen/xenbus/xenbus_xs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/xen/xenbus/xenbus_xs.c b/drivers/xen/xenbus/xenbus_xs.c index 3f3b29398ab8..49a3874ae6bb 100644 --- a/drivers/xen/xenbus/xenbus_xs.c +++ b/drivers/xen/xenbus/xenbus_xs.c @@ -140,7 +140,9 @@ void xs_request_exit(struct xb_req_data *req) spin_lock(&xs_state_lock); xs_state_users--; if ((req->type == XS_TRANSACTION_START && req->msg.type == XS_ERROR) || - req->type == XS_TRANSACTION_END) + (req->type == XS_TRANSACTION_END && +!WARN_ON_ONCE(req->msg.type == XS_ERROR && + !strcmp(req->body, "ENOENT" xs_state_users--; spin_unlock(&xs_state_lock); -- 2.16.2 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] preparations for 4.9.2 and 4.7.5
On Mon, 12 Mar 2018, Julien Grall wrote: > On 12/03/18 10:24, Julien Grall wrote: > > Hi, > > > > On 11/03/18 20:48, Stefano Stabellini wrote: > > > On Wed, 7 Mar 2018, Jan Beulich wrote: > > > > > > > On 06.03.18 at 20:24, wrote: > > > > > On Tue, 6 Mar 2018, Jan Beulich wrote: > > > > > > these stable releases should go out before the end of the month. > > > > > > Please point out backport candidates you find missing from the > > > > > > respective staging branches, but which you consider relevant. > > > > > > Please note that 4.7.5 is expected to be the last xenproject.org > > > > > > managed release from its branch. > > > > > > > > > > I am waiting for master to pass Julien's PSCI 1.1 series, then I > > > > > intend > > > > > to backport it to all stable trees (commits from > > > > > f30b93b42b7137654a69676a61620f763c4ad3b3 to > > > > > cd8b749282475caef095ea2f339a01d1ff9714ae). > > > > > > > > > > Backports to older trees might be difficult. > > > > > > > > > > Given your stable release plan, do you suggest I should start the > > > > > backports now, even if master has not passed yet, or wait? > > > > > > > > There have been a lot of minor issues lately keeping pushes from > > > > happening on master, so if the commits above have not been > > > > pushed just because of such a glitch, I'd be fine with them being > > > > backported right away. If, however, there's any doubt, then I'd > > > > prefer if you waited. But in the end on the ARM side you know > > > > better than me what's best. > > > Master hasn't passed yet, so no backports of the ARM64 Spectre > > > mitigation for the moment. > > > > I really don't like the idea to ship 4.9.2 and 4.7.5 with broken arm64 > > spectre patches. This is indeed the case today as the previous series was > > based on early discussion. > > > > But unstable is blocked on amd64 patches. None of the arm64 spectre > > ^ "amd64 tests". > > > series touch common code, so I am not sure what prevents us to backport > > them. After looking at the test results, which are good for arm, and considering that master hasn't passed yet after 2 more days, I agree with Julien: I think we should not release 4.9.2 and 4.7.5 without the arm64 spectre patches. At this point, I'll proceed to backport the patches now. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 120759: tolerable all pass - PUSHED
flight 120759 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/120759/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 1dd9fa04fae69a2371572663d150c976a9e948d3 baseline version: xen eef83fd2af0d4c78afec34c199c977fc97d8a0b3 Last test of basis 120679 2018-03-13 12:06:56 Z1 days Failing since120685 2018-03-13 17:01:17 Z1 days9 attempts Testing same since 120759 2018-03-14 19:02:40 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Anthony PERARD Doug Goldstein Jan Beulich John Thomson Michael Young Roger Pau Monne Roger Pau Monné 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 eef83fd2af..1dd9fa04fa 1dd9fa04fae69a2371572663d150c976a9e948d3 -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] [v3] xen: remove pre-xen3 fallback handlers
On 03/13/2018 05:06 PM, Arnd Bergmann wrote: > The legacy hypercall handlers were originally added with > a comment explaining that "copying the argument structures in > HYPERVISOR_event_channel_op() and HYPERVISOR_physdev_op() into the local > variable is sufficiently safe" and only made sure to not write > past the end of the argument structure, the checks in linux/string.h > disagree with that, when link-time optimizations are used: > > In function 'memcpy', > inlined from 'pirq_query_unmask' at drivers/xen/fallback.c:53:2, > inlined from '__startup_pirq' at drivers/xen/events/events_base.c:529:2, > inlined from 'restore_pirqs' at drivers/xen/events/events_base.c:1439:3, > inlined from 'xen_irq_resume' at drivers/xen/events/events_base.c:1581:2: > include/linux/string.h:350:3: error: call to '__read_overflow2' declared with > attribute error: detected read beyond size of object passed as 2nd parameter >__read_overflow2(); >^ > > Further research turned out that only Xen 3.0.2 or earlier required the > fallback at all, while all versions in use today don't need it. > As far as I can tell, it is not even possible to run a mainline kernel > on those old Xen releases, at the time when they were in use, only > a patched kernel was supported anyway. > > Fixes: cf47a83fb06e ("xen/hypercall: fix hypercall fallback code for very old > hypervisors") > Signed-off-by: Arnd Bergmann > --- > [v2] use a table lookup instead of a switch/case statement, after > multiple suggestions. > [v3] remove that file completely (+Jan who added this file originally) Reviewed-by: Boris Ostrovsky ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable test] 120626: regressions - FAIL
flight 120626 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/120626/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-livepatch 7 xen-boot fail REGR. vs. 120037 test-amd64-i386-xl-qemuu-debianhvm-amd64 7 xen-boot fail REGR. vs. 120037 test-amd64-i386-xl-qemuu-ovmf-amd64 7 xen-boot fail REGR. vs. 120037 test-amd64-i386-migrupgrade 11 xen-boot/dst_hostfail REGR. vs. 120037 test-amd64-i386-libvirt 7 xen-boot fail REGR. vs. 120037 test-amd64-i386-xl-qemut-debianhvm-amd64 7 xen-boot fail REGR. vs. 120037 test-amd64-i386-libvirt-xsm 8 host-ping-check-xen fail REGR. vs. 120037 test-amd64-i386-xl-qemuu-win7-amd64 7 xen-boot fail REGR. vs. 120037 test-amd64-i386-freebsd10-amd64 7 xen-boot fail REGR. vs. 120037 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 120037 test-amd64-i386-rumprun-i386 7 xen-boot fail REGR. vs. 120037 test-amd64-i386-examine 8 reboot fail REGR. vs. 120037 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 120037 test-amd64-i386-xl-xsm7 xen-boot fail REGR. vs. 120037 test-amd64-i386-xl-qemuu-ws16-amd64 7 xen-boot fail REGR. vs. 120037 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 120037 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail REGR. vs. 120037 test-amd64-i386-xl-qemut-ws16-amd64 7 xen-boot fail REGR. vs. 120037 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 120037 test-amd64-i386-xl7 xen-boot fail REGR. vs. 120037 test-amd64-i386-qemut-rhel6hvm-intel 7 xen-boot fail REGR. vs. 120037 test-amd64-i386-qemuu-rhel6hvm-intel 7 xen-boot fail REGR. vs. 120037 test-amd64-i386-xl-raw7 xen-boot fail REGR. vs. 120037 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 120037 test-amd64-i386-xl-qemut-win10-i386 7 xen-boot fail REGR. vs. 120037 test-amd64-i386-xl-qemut-win7-amd64 7 xen-boot fail REGR. vs. 120037 test-amd64-i386-qemuu-rhel6hvm-amd 7 xen-boot fail REGR. vs. 120037 test-amd64-i386-qemut-rhel6hvm-amd 7 xen-boot fail REGR. vs. 120037 test-amd64-i386-freebsd10-i386 7 xen-boot fail REGR. vs. 120037 test-amd64-i386-pair 11 xen-boot/dst_hostfail REGR. vs. 120037 test-amd64-i386-xl-qemuu-win10-i386 7 xen-boot fail REGR. vs. 120037 test-amd64-amd64-i386-pvgrub 10 debian-di-installfail REGR. vs. 120037 Regressions which are regarded as allowable (not blocking): test-armhf-armhf-xl-rtds16 guest-start/debian.repeat fail REGR. vs. 120037 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 120001 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 120037 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 120037 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 120037 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 120037 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 120037 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail like 120037 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-pvshim 7 xen-boot 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-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-x
[Xen-devel] [linux-4.1 test] 120636: regressions - FAIL
flight 120636 linux-4.1 real [real] http://logs.test-lab.xenproject.org/osstest/logs/120636/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-pvops 6 kernel-build fail REGR. vs. 118294 build-i386-pvops 6 kernel-build fail REGR. vs. 118294 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-arm64-arm64-examine 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-ws16-amd64 1 build-check(1) blocked n/a test-amd64-i386-examine 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-pvshim 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-i386-qemut-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-i386-pair 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-qemut-rhel6hvm-intel 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-intel 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-debianhvm-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-win10-i386 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-raw1 build-check(1) blocked n/a test-amd64-i386-rumprun-i386 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win10-i386 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-win7-amd64 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-xsm1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail like 118294 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 118294 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 118294 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 118294 test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-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-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-
Re: [Xen-devel] [PATCH v2 1/4] x86/PVHv2: Add memory map pointer to hvm_start_info struct
On 3/14/2018 10:28 AM, Boris Ostrovsky wrote: On 03/14/2018 03:55 AM, Jan Beulich wrote: On 14.03.18 at 00:31, wrote: + * For x86 implementations at least, the values used in the type field will + * match the Address Range Types as defined in section 15 (System Address + * Map Interfaces) of the ACPI Specification (http://uefi.org/specifications) + * where: + * AddressRangeMemory = 1 (E820_RAM) + * AddressRangeReserved = 2 (E820_RESERVED) + * AddressRangeACPI = 3 (E820_ACPI) + * AddressRangeNVS = 4 (E820_NVS) + * AddressRangeUnusable = 5 (E820_UNUSABLE) + * AddressRangeDisabled = 6 (E820_DISABLED) + * AddressRangePersistentMemory = 7 (E820_PMEM) Would you mind waiting for a discussion to settle before sending out new patch versions? As indicated in an earlier reply to v1, I consider this still insufficient. And no, I'm not asking for you to add redundant and potentially conflicting definitions of E820_*, but instead you want to use Xen specific ones (prefixed e.g. by XEN_HVM_MEMMAP_TYPE_). Since we will now have a non-Xen user of this interface perhaps PVH_MEMMAP_TYPE_ ? OK, I think I'm following the specifics now. But just to make sure we all on the same page before sending out the next version... I'll be adding something like the following to the header file: ... /* * For x86 implementations at least, the values used in the type field of the * memory map table entries are defined below and match the Address Range Types * as defined in section 15 (System Address Map Interfaces) of the ACPI * Specification (http://uefi.org/specifications) */ #define PVH_MEMMAP_TYPE_RAM 1 #define PVH_MEMMAP_TYPE_RESERVED 2 #define PVH_MEMMAP_TYPE_ACPI 3 #define PVH_MEMMAP_TYPE_NVS 4 #define PVH_MEMMAP_TYPE_UNUSABLE 5 #define PVH_MEMMAP_TYPE_PMEM 7 ... And then we will find an appropriate place in the c code to add a couple of BUILD_BUG_ON() macros to make sure the above remain consistent with E820_xxx. Does that sound about right? Thanks, -Maran -boris ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 10/16] xen/mm: Switch map_pages_to_xen to use MFN typesafe
On 14/03/18 18:20, julien.gr...@arm.com wrote: > From: Julien Grall > > The current prototype is slightly confusing because it takes a virtual > address and a physical frame (not address!). Switching to MFN will improve > safety and reduce the chance to mistakenly invert the 2 parameters. > > Also, take the opportunity to switch (a - b) >> PAGE_SHIFT to > PFN_DOWN(a - b) in the code modified. > > Signed-off-by: Julien Grall Acked-by: Andrew Cooper ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 08/16] xen/mm: Drop the parameter mfn from populate_pt_range
On 14/03/18 18:20, julien.gr...@arm.com wrote: > From: Julien Grall > > The function populate_pt_range is used to populate in advance the > page-table but it will not do the actual mapping. So passing the MFN in > parameter is pointless. Note that the only caller pass 0... > > At the same time replace 0 by INVALID_MFNs. While this does not matter > as the entry will marked as not valid and populated, INVALID_MFN > helps the reader to know the MFN is invalid. > > Signed-off-by: Julien Grall Acked-by: Andrew Cooper ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 09/16] xen/pdx: Introduce helper to convert MFN <-> PDX
On 14/03/18 18:20, julien.gr...@arm.com wrote: > From: Julien Grall > > This will avoid use of pfn_to_pdx(mfn_x(mfn)) over the code base. > > Signed-off-by: Julien Grall > Reviewed-by: Wei Liu Acked-by: Andrew Cooper ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 01/16] x86/mm: skip incrementing mfn if it is not a valid mfn
On 14/03/18 18:19, julien.gr...@arm.com wrote: > From: Wei Liu > > The function is called to fill in page table entries in > populate_pt_range. Skip incrementing mfn if it is invalid. > > Signed-off-by: Wei Liu Remind me what the purpose of this patch is? ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 6/6] ARM: GIC: extend LR read/write functions to cover EOI and source
On 03/14/2018 06:19 PM, Andre Przywara wrote: Hi, Hi Andre, Thank you for the review. On 09/03/18 16:35, julien.gr...@arm.com wrote: From: Andre Przywara I think this is quite different from what I ever wrote, so please drop my authorship here. Fine, I wasn't sure given that you were the original author or extending the LR. I can pointing that in the commit message :). So far our LR read/write functions do not handle the EOI bit and the source CPUID bits in an LR, because the current VGIC implementation does not use them. Extend the gic_lr data structure to hold these bits of information by using a union to differentiate field used depending on whether the vIRQ has a corresponding pIRQ. As mentioned before, I am not sure this is really necessary. The idea of struct gic_lr is to provide a hardware agnostic view of an LR. So the actual read_lr/write_lr function take care of reading/populating pINTID, iff the HW bit is set (as done in your patch 5/6). Given that, I don't think we need to change the current code in this respect, as this is just a small internal interface. Even if I know the vGIC, I find easier with this solution to differentiate what is for the HW IRQ or not. The size of Xen Arm is becoming quite significant for me to remember every single line/structure. So the main goal here is to help the reviewer to spend less time on patch review as it helps to spot directly misusage. But then again I don't care enough, so if that makes you happy Note that source is not covered by GICv3 LR. I was thinking the same, but Marc pointed me to that inconspicuous note on page 8-299 in ARM IHI 0069D (at the end of "vINTID", chapter 8.4.6). Doh :/. I will drop the comment and update the GICv3 code then. This allows the new VGIC to use this information. Signed-off-by: Andre Przywara Signed-off-by: Julien Grall --- xen/arch/arm/gic-v2.c | 22 +++--- xen/arch/arm/gic-v3.c | 11 +-- xen/include/asm-arm/gic.h | 16 ++-- 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c index daf8c61258..69f8d6044a 100644 --- a/xen/arch/arm/gic-v2.c +++ b/xen/arch/arm/gic-v2.c @@ -474,8 +474,17 @@ static void gicv2_read_lr(int lr, struct gic_lr *lr_reg) if ( lr_reg->hw_status ) { -lr_reg->pirq = lrv >> GICH_V2_LR_PHYSICAL_SHIFT; -lr_reg->pirq &= GICH_V2_LR_PHYSICAL_MASK; +lr_reg->h.pirq = lrv >> GICH_V2_LR_PHYSICAL_SHIFT; +lr_reg->h.pirq &= GICH_V2_LR_PHYSICAL_MASK; +} +else +{ +lr_reg->v.eoi = (lrv & GICH_V2_LR_MAINTENANCE_IRQ) == GICH_V2_LR_MAINTENANCE_IRQ; I think you can drop the " == GICH_V2_LR_MAINTENANCE_IRQ", as .eoi is a bool. Avoids the linebreak. The '==' was more to avoid assuming GIC_V2_LR_MAINTENANCE_IRQ is a single bit. But I was probably too cautious, I will drop it. writel_gich(lrv, GICH_LR + lr * 4); diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c index f73d386df1..a855069111 100644 --- a/xen/arch/arm/gic-v3.c +++ b/xen/arch/arm/gic-v3.c @@ -1014,7 +1014,9 @@ static void gicv3_read_lr(int lr, struct gic_lr *lr_reg) lr_reg->hw_status = (lrv & ICH_LR_HW) == ICH_LR_HW; if ( lr_reg->hw_status ) -lr_reg->pirq = (lrv >> ICH_LR_PHYSICAL_SHIFT) & ICH_LR_PHYSICAL_MASK; +lr_reg->h.pirq = (lrv >> ICH_LR_PHYSICAL_SHIFT) & ICH_LR_PHYSICAL_MASK; +else +lr_reg->v.eoi = (lrv & ICH_LR_MAINTENANCE_IRQ) == ICH_LR_MAINTENANCE_IRQ; Same here. Ditto. } static void gicv3_write_lr(int lr_reg, const struct gic_lr *lr) @@ -1033,7 +1035,12 @@ static void gicv3_write_lr(int lr_reg, const struct gic_lr *lr) if ( lr->hw_status ) { lrv |= ICH_LR_HW; -lrv |= (uint64_t)lr->pirq << ICH_LR_PHYSICAL_SHIFT; +lrv |= (uint64_t)lr->h.pirq << ICH_LR_PHYSICAL_SHIFT; +} +else +{ +if ( lr->v.eoi ) +lrv |= ICH_LR_MAINTENANCE_IRQ; } /* diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h index 545901b120..4cf5bb385d 100644 --- a/xen/include/asm-arm/gic.h +++ b/xen/include/asm-arm/gic.h @@ -204,14 +204,26 @@ union gic_state_data { * The LR register format is different for GIC HW version */ struct gic_lr { - /* Physical IRQ -> Only set when hw_status is set. */ - uint32_t pirq; /* Virtual IRQ */ uint32_t virq; uint8_t priority; bool active; bool pending; bool hw_status; + union + { + /* Only filled when there are a corresponding pIRQ (hw_state = true) */ + struct + { + uint32_t pirq; + } h; + /* Only filled when there are no corresponding pIRQ (hw_state = false) */ + struct + { + bool eoi; + uint8_t source; /* GICv2 only */ + } v; That looks somewhat confusing to me. So either you use "hw" and "virt" for the struct identifiers,
[Xen-devel] [PATCH v5 05/16] xen/arm: mm: Remove unused relinquish_shared_pages
From: Julien Grall relinquish_shared_pages is never called on Arm. Signed-off-by: Julien Grall --- Cc: Stefano Stabellini Changes in v4: - Patch added --- xen/include/asm-arm/mm.h | 4 1 file changed, 4 deletions(-) diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h index c03f4ad674..7678e29c15 100644 --- a/xen/include/asm-arm/mm.h +++ b/xen/include/asm-arm/mm.h @@ -321,10 +321,6 @@ struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va, unsigned long flags); static inline void put_gfn(struct domain *d, unsigned long gfn) {} -static inline int relinquish_shared_pages(struct domain *d) -{ -return 0; -} #define INVALID_M2P_ENTRY(~0UL) #define SHARED_M2P_ENTRY (~0UL - 1UL) -- 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] 120751: regressions - FAIL
flight 120751 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/120751/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-xsm 6 xen-buildfail REGR. vs. 120679 build-armhf 6 xen-buildfail REGR. vs. 120679 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass version targeted for testing: xen 29036baa23da7e82896262cbcbffdbc8faf98147 baseline version: xen eef83fd2af0d4c78afec34c199c977fc97d8a0b3 Last test of basis 120679 2018-03-13 12:06:56 Z1 days Failing since120685 2018-03-13 17:01:17 Z1 days8 attempts Testing same since 120751 2018-03-14 16:01:52 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Anthony PERARD Doug Goldstein Jan Beulich John Thomson Michael Young Roger Pau Monne Roger Pau Monné Wei Liu jobs: build-arm64-xsm fail build-amd64 pass build-armhf fail build-amd64-libvirt pass test-armhf-armhf-xl blocked test-arm64-arm64-xl-xsm blocked 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 Not pushing. commit 29036baa23da7e82896262cbcbffdbc8faf98147 Author: Andrew Cooper Date: Wed Mar 14 10:36:09 2018 + x86/entry: Trivial nonfunctional fixes * Drop unnecessary size suffixes * The C pseudocode refers to a trap_info object, not trap_bounce. Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich commit dedbdcde95ce0dc3f1a51ad9c685a71570630a7d Author: Andrew Cooper Date: Wed Mar 14 10:48:36 2018 + x86/pv: Fix guest crashes following f75b1a5247b "x86/pv: Drop int80_bounce from struct pv_vcpu" The original init_int80_direct_trap() was in fact buggy; `int $0x80` is not an exception. This went unnoticed for years because int80_bounce and trap_bounce were separate structures, but were combined by this change. Exception handling is different to interrupt handling for PV guests. By reusing trap_bounce, the following corner case can occur: * Handle a guest `int $0x80` instruction. Latches TBF_EXCEPTION into trap_bounce. * Handle an exception, which emulates to success (such as ptwr support), which leaves trap_bounce unmodified. * The exception exit path sees TBF_EXCEPTION set and re-injects the `int $0x80` a second time. Drop the TBF_EXCEPTION from the int80 invocation, which matches the equivalent logic from the syscall/sysenter paths. Reported-by: Sander Eikelenboom Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich commit db0c7dde021c29c2ae0d847d70fb7b59e02ea522 Author: Anthony PERARD Date: Tue Mar 13 11:13:18 2018 + libxl_qmp: Tell QEMU about live migration or snapshot Since version 2.10, QEMU will lock the disk images so a second QEMU instance will not try to open it. This would prevent live migration from working correctly. A new parameter as been added to the QMP command "xen-save-devices-state" in QEMU version 2.11 which allow to unlock the disk image for a live migration, but also keep it locked for a snapshot. QEMU commit: 5d6c599fe1d69a1bf8c5c4d3c58be2b31cd625ad "migration, xen: Fix block image lock issue on live migration" The extra "live" parameter can only be use if QEMU knows about it, so only add it if qemu is recent enough. The struct libxl__domain_suspend_state as now knowledge if the suspend is part of a live migration. Signed-off-by: Anthony PERARD Acked-by: Wei Liu commit ab7
[Xen-devel] [PATCH v5 15/16] xen/x86: Switch mfn_to_page in x86_64/mm.c to use typesafe MFN
From: Julien Grall Other than _mfn(0) -> INVALID_MFN, no functional change intendend. Signed-off-by: Julien Grall Acked-by: Jan Beulich --- Cc: Jan Beulich Cc: Andrew Cooper Changes in v5: - Use INVALID_MFN instead of _mfn(0) Changes in v4: - Patch added --- xen/arch/x86/x86_64/mm.c | 46 ++ 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c index e03250bcdd..a54e2c9be4 100644 --- a/xen/arch/x86/x86_64/mm.c +++ b/xen/arch/x86/x86_64/mm.c @@ -43,6 +43,8 @@ asm(".file \"" __FILE__ "\""); /* Override macros from asm/page.h to make them work with mfn_t */ #undef page_to_mfn #define page_to_mfn(pg) _mfn(__page_to_mfn(pg)) +#undef mfn_to_page +#define mfn_to_page(mfn) __mfn_to_page(mfn_x(mfn)) unsigned int __read_mostly m2p_compat_vstart = __HYPERVISOR_COMPAT_VIRT_START; @@ -160,7 +162,8 @@ static int m2p_mapped(unsigned long spfn) static int share_hotadd_m2p_table(struct mem_hotadd_info *info) { -unsigned long i, n, v, m2p_start_mfn = 0; +unsigned long i, n, v; +mfn_t m2p_start_mfn = INVALID_MFN; l3_pgentry_t l3e; l2_pgentry_t l2e; @@ -180,15 +183,16 @@ static int share_hotadd_m2p_table(struct mem_hotadd_info *info) l2e = l3e_to_l2e(l3e)[l2_table_offset(v)]; if ( !(l2e_get_flags(l2e) & _PAGE_PRESENT) ) continue; -m2p_start_mfn = l2e_get_pfn(l2e); +m2p_start_mfn = l2e_get_mfn(l2e); } else continue; for ( i = 0; i < n; i++ ) { -struct page_info *page = mfn_to_page(m2p_start_mfn + i); -if (hotadd_mem_valid(m2p_start_mfn + i, info)) +struct page_info *page = mfn_to_page(mfn_add(m2p_start_mfn, i)); + +if ( hotadd_mem_valid(mfn_x(mfn_add(m2p_start_mfn, i)), info) ) share_xen_page_with_privileged_guests(page, XENSHARE_readonly); } } @@ -204,12 +208,13 @@ static int share_hotadd_m2p_table(struct mem_hotadd_info *info) l2e = l3e_to_l2e(l3e)[l2_table_offset(v)]; if ( !(l2e_get_flags(l2e) & _PAGE_PRESENT) ) continue; -m2p_start_mfn = l2e_get_pfn(l2e); +m2p_start_mfn = l2e_get_mfn(l2e); for ( i = 0; i < L1_PAGETABLE_ENTRIES; i++ ) { -struct page_info *page = mfn_to_page(m2p_start_mfn + i); -if (hotadd_mem_valid(m2p_start_mfn + i, info)) +struct page_info *page = mfn_to_page(mfn_add(m2p_start_mfn, i)); + +if ( hotadd_mem_valid(mfn_x(mfn_add(m2p_start_mfn, i)), info) ) share_xen_page_with_privileged_guests(page, XENSHARE_readonly); } } @@ -720,10 +725,10 @@ static void cleanup_frame_table(struct mem_hotadd_info *info) unsigned long sva, eva; l3_pgentry_t l3e; l2_pgentry_t l2e; -unsigned long spfn, epfn; +mfn_t spfn, epfn; -spfn = info->spfn; -epfn = info->epfn; +spfn = _mfn(info->spfn); +epfn = _mfn(info->epfn); sva = (unsigned long)mfn_to_page(spfn); eva = (unsigned long)mfn_to_page(epfn); @@ -795,16 +800,17 @@ static int setup_frametable_chunk(void *start, void *end, static int extend_frame_table(struct mem_hotadd_info *info) { -unsigned long cidx, nidx, eidx, spfn, epfn; +unsigned long cidx, nidx, eidx; +mfn_t spfn, epfn; -spfn = info->spfn; -epfn = info->epfn; +spfn = _mfn(info->spfn); +epfn = _mfn(info->epfn); -eidx = (pfn_to_pdx(epfn) + PDX_GROUP_COUNT - 1) / PDX_GROUP_COUNT; -nidx = cidx = pfn_to_pdx(spfn)/PDX_GROUP_COUNT; +eidx = (mfn_to_pdx(epfn) + PDX_GROUP_COUNT - 1) / PDX_GROUP_COUNT; +nidx = cidx = mfn_to_pdx(spfn)/PDX_GROUP_COUNT; -ASSERT( pfn_to_pdx(epfn) <= (DIRECTMAP_SIZE >> PAGE_SHIFT) && -pfn_to_pdx(epfn) <= FRAMETABLE_NR ); +ASSERT( mfn_to_pdx(epfn) <= (DIRECTMAP_SIZE >> PAGE_SHIFT) && +mfn_to_pdx(epfn) <= FRAMETABLE_NR ); if ( test_bit(cidx, pdx_group_valid) ) cidx = find_next_zero_bit(pdx_group_valid, eidx, cidx); @@ -866,7 +872,7 @@ void __init subarch_init_memory(void) for ( i = 0; i < n; i++ ) { -struct page_info *page = mfn_to_page(m2p_start_mfn + i); +struct page_info *page = mfn_to_page(_mfn(m2p_start_mfn + i)); share_xen_page_with_privileged_guests(page, XENSHARE_readonly); } } @@ -886,7 +892,7 @@ void __init subarch_init_memory(void) for ( i = 0; i < L1_PAGETABLE_ENTRIES; i++ ) { -struct page_info *page = mfn_to_page(m2p_start_mfn + i); +struct page_info *page = mfn_to_page(_mfn(m2p_start_mfn + i)); share_xen_page_with_privileged_guests(page, XENSHARE_readonly); } } @@ -1274,7 +1280,7 @@ static int transfer_pages_to_heap(struct mem_hotadd_info *info) */ for (i = info->spfn; i
[Xen-devel] [PATCH v5 16/16] xen: Convert page_to_mfn and mfn_to_page to use typesafe MFN
From: Julien Grall Most of the users of page_to_mfn and mfn_to_page are either overriding the macros to make them work with mfn_t or use mfn_x/_mfn because the rest of the function use mfn_t. So make page_to_mfn and mfn_to_page return mfn_t by default. The __* version are now dropped as this patch will convert all the remaining non-typesafe callers. Only reasonable clean-ups are done in this patch. The rest will use _mfn/mfn_x for the time being. Lastly, domain_page_to_mfn is also converted to use mfn_t given that most of the callers are now switched to _mfn(domain_page_to_mfn(...)). Signed-off-by: Julien Grall Acked-by: Razvan Cojocaru Reviewed-by: Paul Durrant Reviewed-by: Boris Ostrovsky Reviewed-by: Kevin Tian Reviewed-by: Wei Liu --- Andrew suggested to drop IS_VALID_PAGE in xen/tmem_xen.h. His comment was: "/sigh This is tautological. The definition of a "valid mfn" in this case is one for which we have frametable entry, and by having a struct page_info in our hands, this is by definition true (unless you have a wild pointer, at which point your bug is elsewhere). IS_VALID_PAGE() is only ever used in assertions and never usefully, so instead I would remove it entirely rather than trying to fix it up." I can remove the function in a separate patch at the begining of the series if Konrad (TMEM maintainer) is happy with that. Cc: Stefano Stabellini Cc: Julien Grall Cc: Andrew Cooper Cc: George Dunlap Cc: Ian Jackson Cc: Jan Beulich Cc: Konrad Rzeszutek Wilk Cc: Tim Deegan Cc: Tamas K Lengyel Cc: Suravee Suthikulpanit Cc: Jun Nakajima Cc: George Dunlap Cc: Gang Wei Cc: Shane Wang Changes in v5: - Remove some spurious parentheses in the code changed - Remove spurious change in _set_gpfn_from_mfn - Add Razvan's acked-by - Add Paul's reviewed-by - Add Boris's reviewed-by - Add Kevin's reviewed-by - Add Wei's reviewed-by Changes in v4: - Drop __page_to_mfn and __mfn_to_page. Reword the commit title/message to reflect that. Changes in v3: - Rebase on the latest staging and fix some conflicts. Tags haven't be retained. - Switch the printf format to PRI_mfn Changes in v2: - Some part have been moved in separate patch - Remove one spurious comment - Convert domain_page_to_mfn to use mfn_t --- xen/arch/arm/domain_build.c | 2 -- xen/arch/arm/kernel.c | 2 +- xen/arch/arm/mem_access.c | 2 +- xen/arch/arm/mm.c | 8 xen/arch/arm/p2m.c | 10 ++ xen/arch/x86/cpu/vpmu.c | 4 ++-- xen/arch/x86/domain.c | 21 +++-- xen/arch/x86/domain_page.c | 6 +++--- xen/arch/x86/hvm/dm.c | 2 +- xen/arch/x86/hvm/dom0_build.c | 6 +++--- xen/arch/x86/hvm/emulate.c | 6 +++--- xen/arch/x86/hvm/hvm.c | 12 ++-- xen/arch/x86/hvm/ioreq.c| 4 ++-- xen/arch/x86/hvm/stdvga.c | 2 +- xen/arch/x86/hvm/svm/svm.c | 4 ++-- xen/arch/x86/hvm/viridian.c | 6 +++--- xen/arch/x86/hvm/vmx/vmcs.c | 2 +- xen/arch/x86/hvm/vmx/vmx.c | 10 +- xen/arch/x86/hvm/vmx/vvmx.c | 6 +++--- xen/arch/x86/mm.c | 4 xen/arch/x86/mm/guest_walk.c| 6 +++--- xen/arch/x86/mm/hap/guest_walk.c| 2 +- xen/arch/x86/mm/hap/hap.c | 6 -- xen/arch/x86/mm/hap/nested_ept.c| 2 +- xen/arch/x86/mm/mem_sharing.c | 5 - xen/arch/x86/mm/p2m-ept.c | 8 xen/arch/x86/mm/p2m-pod.c | 6 -- xen/arch/x86/mm/p2m.c | 6 -- xen/arch/x86/mm/paging.c| 6 -- xen/arch/x86/mm/shadow/private.h| 16 ++-- xen/arch/x86/numa.c | 2 +- xen/arch/x86/physdev.c | 2 +- xen/arch/x86/pv/callback.c | 6 -- xen/arch/x86/pv/descriptor-tables.c | 6 -- xen/arch/x86/pv/dom0_build.c| 14 +++--- xen/arch/x86/pv/domain.c| 6 -- xen/arch/x86/pv/emul-gate-op.c | 6 -- xen/arch/x86/pv/emul-priv-op.c | 10 -- xen/arch/x86/pv/grant_table.c | 6 -- xen/arch/x86/pv/ro-page-fault.c | 6 -- xen/arch/x86/pv/shim.c | 4 +--- xen/arch/x86/smpboot.c | 6 -- xen/arch/x86/tboot.c| 4 ++-- xen/arch/x86/traps.c| 4 ++-- xen/arch/x86/x86_64/mm.c| 6 -- xen/common/domain.c | 4 ++-- xen/common/grant_table.c| 6 -- xen/common/kimage.c | 6 -- xen/common/memory.c | 6 -- xen
[Xen-devel] [PATCH v5 07/16] xen/x86: mm: Switch x86/mm.c to use typesafe for virt_to_mfn
From: Julien Grall No functional change intended. Signed-off Julien Grall Acked-by: Jan Beulich --- Cc: Jan Beulich Cc: Andrew Cooper Changes in v5: - Add Jan's acked-by - Use PFN_DOWN Changes in v4: - Patch added --- xen/arch/x86/mm.c | 17 + 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 5f5577c7c2..ab10f552ea 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -135,6 +135,8 @@ #define mfn_to_page(mfn) __mfn_to_page(mfn_x(mfn)) #undef page_to_mfn #define page_to_mfn(pg) _mfn(__page_to_mfn(pg)) +#undef virt_to_mfn +#define virt_to_mfn(v) _mfn(__virt_to_mfn(v)) /* Mapping of the fixmap space needed early. */ l1_pgentry_t __section(".bss.page_aligned") __aligned(PAGE_SIZE) @@ -380,7 +382,7 @@ void __init arch_init_memory(void) l3tab[i] = l3idle[i]; for ( ; i < L3_PAGETABLE_ENTRIES; ++i ) l3tab[i] = l3e_empty(); -split_l4e = l4e_from_pfn(virt_to_mfn(l3tab), +split_l4e = l4e_from_mfn(virt_to_mfn(l3tab), __PAGE_HYPERVISOR_RW); } else @@ -4155,7 +4157,7 @@ int xenmem_add_to_physmap_one( { case XENMAPSPACE_shared_info: if ( idx == 0 ) -mfn = _mfn(virt_to_mfn(d->shared_info)); +mfn = virt_to_mfn(d->shared_info); break; case XENMAPSPACE_grant_table: rc = gnttab_map_frame(d, idx, gpfn, &mfn); @@ -4781,7 +4783,7 @@ int map_pages_to_xen( if ( (l3e_get_flags(*pl3e) & _PAGE_PRESENT) && (l3e_get_flags(*pl3e) & _PAGE_PSE) ) { -l3e_write_atomic(pl3e, l3e_from_pfn(virt_to_mfn(pl2e), +l3e_write_atomic(pl3e, l3e_from_mfn(virt_to_mfn(pl2e), __PAGE_HYPERVISOR)); pl2e = NULL; } @@ -4879,7 +4881,7 @@ int map_pages_to_xen( if ( (l2e_get_flags(*pl2e) & _PAGE_PRESENT) && (l2e_get_flags(*pl2e) & _PAGE_PSE) ) { -l2e_write_atomic(pl2e, l2e_from_pfn(virt_to_mfn(pl1e), +l2e_write_atomic(pl2e, l2e_from_mfn(virt_to_mfn(pl1e), __PAGE_HYPERVISOR)); pl1e = NULL; } @@ -5088,7 +5090,7 @@ int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int nf) if ( (l3e_get_flags(*pl3e) & _PAGE_PRESENT) && (l3e_get_flags(*pl3e) & _PAGE_PSE) ) { -l3e_write_atomic(pl3e, l3e_from_pfn(virt_to_mfn(pl2e), +l3e_write_atomic(pl3e, l3e_from_mfn(virt_to_mfn(pl2e), __PAGE_HYPERVISOR)); pl2e = NULL; } @@ -5142,7 +5144,7 @@ int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int nf) if ( (l2e_get_flags(*pl2e) & _PAGE_PRESENT) && (l2e_get_flags(*pl2e) & _PAGE_PSE) ) { -l2e_write_atomic(pl2e, l2e_from_pfn(virt_to_mfn(pl1e), +l2e_write_atomic(pl2e, l2e_from_mfn(virt_to_mfn(pl1e), __PAGE_HYPERVISOR)); pl1e = NULL; } @@ -5546,8 +5548,7 @@ static void __memguard_change_range(void *p, unsigned long l, int guard) if ( guard ) flags &= ~_PAGE_PRESENT; -map_pages_to_xen( -_p, virt_to_maddr(p) >> PAGE_SHIFT, _l >> PAGE_SHIFT, flags); +map_pages_to_xen(_p, mfn_x(virt_to_mfn(p)), PFN_DOWN(_l), flags); } void memguard_guard_range(void *p, unsigned long l) -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v5 12/16] xen/mm: Switch common/memory.c to use typesafe MFN
From: Julien Grall A new helper copy_mfn_to_guest is introduced to easily to copy a MFN to the guest memory. Not functional change intended Signed-off-by: Julien Grall --- Cc: Andrew Cooper Cc: George Dunlap Cc: Ian Jackson Cc: Jan Beulich Cc: Konrad Rzeszutek Wilk Cc: Stefano Stabellini Cc: Tim Deegan Cc: Wei Liu Changes in v5: - Restrict the scope of some mfn variable. Changes in v4: - Patch added --- xen/common/memory.c | 75 - 1 file changed, 46 insertions(+), 29 deletions(-) diff --git a/xen/common/memory.c b/xen/common/memory.c index 3ed71f8f74..01f1d2dbc3 100644 --- a/xen/common/memory.c +++ b/xen/common/memory.c @@ -33,6 +33,12 @@ #include #endif +/* Override macros from asm/page.h to make them work with mfn_t */ +#undef page_to_mfn +#define page_to_mfn(pg) _mfn(__page_to_mfn(pg)) +#undef mfn_to_page +#define mfn_to_page(mfn) __mfn_to_page(mfn_x(mfn)) + struct memop_args { /* INPUT */ struct domain *domain; /* Domain to be affected. */ @@ -95,11 +101,17 @@ static unsigned int max_order(const struct domain *d) return min(order, MAX_ORDER + 0U); } +/* Helper to copy a typesafe MFN to guest */ +#define copy_mfn_to_guest(hnd, off, mfn)\ +({ \ +xen_pfn_t mfn_ = mfn_x(mfn);\ +__copy_to_guest_offset(hnd, off, &mfn_, 1); \ +}) + static void increase_reservation(struct memop_args *a) { struct page_info *page; unsigned long i; -xen_pfn_t mfn; struct domain *d = a->domain; if ( !guest_handle_is_null(a->extent_list) && @@ -132,8 +144,9 @@ static void increase_reservation(struct memop_args *a) if ( !paging_mode_translate(d) && !guest_handle_is_null(a->extent_list) ) { -mfn = page_to_mfn(page); -if ( unlikely(__copy_to_guest_offset(a->extent_list, i, &mfn, 1)) ) +mfn_t mfn = page_to_mfn(page); + +if ( unlikely(copy_mfn_to_guest(a->extent_list, i, mfn)) ) goto out; } } @@ -146,7 +159,7 @@ static void populate_physmap(struct memop_args *a) { struct page_info *page; unsigned int i, j; -xen_pfn_t gpfn, mfn; +xen_pfn_t gpfn; struct domain *d = a->domain, *curr_d = current->domain; bool need_tlbflush = false; uint32_t tlbflush_timestamp = 0; @@ -182,6 +195,8 @@ static void populate_physmap(struct memop_args *a) for ( i = a->nr_done; i < a->nr_extents; i++ ) { +mfn_t mfn; + if ( i != a->nr_done && hypercall_preempt_check() ) { a->preempted = 1; @@ -205,14 +220,15 @@ static void populate_physmap(struct memop_args *a) { if ( is_domain_direct_mapped(d) ) { -mfn = gpfn; +mfn = _mfn(gpfn); -for ( j = 0; j < (1U << a->extent_order); j++, mfn++ ) +for ( j = 0; j < (1U << a->extent_order); j++, + mfn = mfn_add(mfn, 1) ) { -if ( !mfn_valid(_mfn(mfn)) ) +if ( !mfn_valid(mfn) ) { -gdprintk(XENLOG_INFO, "Invalid mfn %#"PRI_xen_pfn"\n", - mfn); +gdprintk(XENLOG_INFO, "Invalid mfn %#"PRI_mfn"\n", + mfn_x(mfn)); goto out; } @@ -220,14 +236,14 @@ static void populate_physmap(struct memop_args *a) if ( !get_page(page, d) ) { gdprintk(XENLOG_INFO, - "mfn %#"PRI_xen_pfn" doesn't belong to d%d\n", - mfn, d->domain_id); + "mfn %#"PRI_mfn" doesn't belong to d%d\n", + mfn_x(mfn), d->domain_id); goto out; } put_page(page); } -mfn = gpfn; +mfn = _mfn(gpfn); } else { @@ -253,15 +269,15 @@ static void populate_physmap(struct memop_args *a) mfn = page_to_mfn(page); } -guest_physmap_add_page(d, _gfn(gpfn), _mfn(mfn), a->extent_order); +guest_physmap_add_page(d, _gfn(gpfn), mfn, a->extent_order); if ( !paging_mode_translate(d) ) { for ( j = 0; j < (1U << a->extent_order); j++ ) -set_gpfn_from_mfn(mfn + j, gpfn + j); +set_gpfn_from_mfn(mfn_x(mfn_add(mfn, j)), gpfn + j); /* Inform the domain of the new page's machine address. */ -if ( unlikely(__copy_to_guest_offset(a->extent_list, i, &mfn, 1)) ) +if ( unlikely(copy_mfn_to_gu
[Xen-devel] [PATCH v5 10/16] xen/mm: Switch map_pages_to_xen to use MFN typesafe
From: Julien Grall The current prototype is slightly confusing because it takes a virtual address and a physical frame (not address!). Switching to MFN will improve safety and reduce the chance to mistakenly invert the 2 parameters. Also, take the opportunity to switch (a - b) >> PAGE_SHIFT to PFN_DOWN(a - b) in the code modified. Signed-off-by: Julien Grall --- Cc: Stefano Stabellini Cc: Julien Grall Cc: Andrew Cooper Cc: George Dunlap Cc: Ian Jackson Cc: Jan Beulich Cc: Konrad Rzeszutek Wilk Cc: Tim Deegan Cc: Wei Liu Cc: Gang Wei Cc: Shane Wang Cc: Kevin Tian Changes in v5: - Use PFN_DOWN as suggested by Jan - Replace _mfn(0) by INVALID_MFN where relevant Changes in v4: - Patch added --- xen/arch/arm/mm.c | 4 +-- xen/arch/x86/mm.c | 58 +++--- xen/arch/x86/setup.c | 20 ++--- xen/arch/x86/smpboot.c | 2 +- xen/arch/x86/tboot.c | 11 xen/arch/x86/x86_64/mm.c | 27 ++ xen/arch/x86/x86_64/mmconfig_64.c | 6 ++-- xen/common/efi/boot.c | 2 +- xen/common/vmap.c | 10 +-- xen/drivers/acpi/apei/erst.c | 2 +- xen/drivers/acpi/apei/hest.c | 2 +- xen/drivers/passthrough/vtd/dmar.c | 2 +- xen/include/asm-arm/mm.h | 2 +- xen/include/xen/mm.h | 2 +- 14 files changed, 80 insertions(+), 70 deletions(-) diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 97dcdd5d50..f17907ace8 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -1065,11 +1065,11 @@ out: } int map_pages_to_xen(unsigned long virt, - unsigned long mfn, + mfn_t mfn, unsigned long nr_mfns, unsigned int flags) { -return create_xen_entries(INSERT, virt, _mfn(mfn), nr_mfns, flags); +return create_xen_entries(INSERT, virt, mfn, nr_mfns, flags); } int populate_pt_range(unsigned long virt, unsigned long nr_mfns) diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 5e3e870260..2d73232ede 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -213,7 +213,7 @@ static void __init init_frametable_chunk(void *start, void *end) while ( step && s + (step << PAGE_SHIFT) > e + (4 << PAGE_SHIFT) ) step >>= PAGETABLE_ORDER; mfn = alloc_boot_pages(step, step); -map_pages_to_xen(s, mfn_x(mfn), step, PAGE_HYPERVISOR); +map_pages_to_xen(s, mfn, step, PAGE_HYPERVISOR); } memset(start, 0, end - start); @@ -793,12 +793,12 @@ static int update_xen_mappings(unsigned long mfn, unsigned int cacheattr) XEN_VIRT_START + ((mfn - PFN_DOWN(xen_phys_start)) << PAGE_SHIFT); if ( unlikely(alias) && cacheattr ) -err = map_pages_to_xen(xen_va, mfn, 1, 0); +err = map_pages_to_xen(xen_va, _mfn(mfn), 1, 0); if ( !err ) -err = map_pages_to_xen((unsigned long)mfn_to_virt(mfn), mfn, 1, +err = map_pages_to_xen((unsigned long)mfn_to_virt(mfn), _mfn(mfn), 1, PAGE_HYPERVISOR | cacheattr_to_pte_flags(cacheattr)); if ( unlikely(alias) && !cacheattr && !err ) -err = map_pages_to_xen(xen_va, mfn, 1, PAGE_HYPERVISOR); +err = map_pages_to_xen(xen_va, _mfn(mfn), 1, PAGE_HYPERVISOR); return err; } @@ -4651,7 +4651,7 @@ l1_pgentry_t *virt_to_xen_l1e(unsigned long v) int map_pages_to_xen( unsigned long virt, -unsigned long mfn, +mfn_t mfn, unsigned long nr_mfns, unsigned int flags) { @@ -4683,13 +4683,13 @@ int map_pages_to_xen( ol3e = *pl3e; if ( cpu_has_page1gb && - !(((virt >> PAGE_SHIFT) | mfn) & + !(((virt >> PAGE_SHIFT) | mfn_x(mfn)) & ((1UL << (L3_PAGETABLE_SHIFT - PAGE_SHIFT)) - 1)) && nr_mfns >= (1UL << (L3_PAGETABLE_SHIFT - PAGE_SHIFT)) && !(flags & (_PAGE_PAT | MAP_SMALL_PAGES)) ) { /* 1GB-page mapping. */ -l3e_write_atomic(pl3e, l3e_from_pfn(mfn, l1f_to_lNf(flags))); +l3e_write_atomic(pl3e, l3e_from_mfn(mfn, l1f_to_lNf(flags))); if ( (l3e_get_flags(ol3e) & _PAGE_PRESENT) ) { @@ -4733,8 +4733,8 @@ int map_pages_to_xen( } virt+= 1UL << L3_PAGETABLE_SHIFT; -if ( !mfn_eq(_mfn(mfn), INVALID_MFN) ) -mfn += 1UL << (L3_PAGETABLE_SHIFT - PAGE_SHIFT); +if ( !mfn_eq(mfn, INVALID_MFN) ) +mfn = mfn_add(mfn, 1UL << (L3_PAGETABLE_SHIFT - PAGE_SHIFT)); nr_mfns -= 1UL << (L3_PAGETABLE_SHIFT - PAGE_SHIFT); continue; } @@ -4749,18 +4749,18 @@ int map_pages_to_xen( if ( ((l3e_get_pfn(ol3e) & ~(L2_PAGETABLE_ENTRIES * L1_PAGETABLE_ENTRIES - 1)) + (l2_table_offset(virt) << PAGETABLE_
[Xen-devel] [PATCH v5 14/16] xen/grant: Switch common/grant_table.c to use typesafe MFN
From: Julien Grall At the same time replace _mfn(0) by INVALID_MFN or drop the initializer when it is not necessary. This will make clearer that the MFN initialized is not valid. Other than _mfn(0) -> INVALID_MFN, no functional change intended. Signed-off-by: Julien Grall --- Cc: Stefano Stabellini Cc: Julien Grall Cc: Andrew Cooper Cc: George Dunlap Cc: Ian Jackson Cc: Jan Beulich Cc: Konrad Rzeszutek Wilk Cc: Tim Deegan Cc: Wei Liu Changes in v5: - Remove _mfn(0) when not needed or replace by INVALID_MFN. Changes in v4: - Patch added --- xen/arch/arm/mm.c | 2 +- xen/common/grant_table.c | 147 -- xen/include/asm-arm/grant_table.h | 2 +- xen/include/asm-x86/grant_table.h | 2 +- 4 files changed, 82 insertions(+), 71 deletions(-) diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 4268dd5c2d..db74466a16 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -1413,7 +1413,7 @@ void gnttab_clear_flag(unsigned long nr, uint16_t *addr) } while (cmpxchg(addr, old, old & mask) != old); } -void gnttab_mark_dirty(struct domain *d, unsigned long l) +void gnttab_mark_dirty(struct domain *d, mfn_t mfn) { /* XXX: mark dirty */ static int warning; diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index f9e3d1bb95..4bedf5984a 100644 --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -40,6 +40,12 @@ #include #include +/* Override macros from asm/page.h to make them work with mfn_t */ +#undef page_to_mfn +#define page_to_mfn(pg) _mfn(__page_to_mfn(pg)) +#undef mfn_to_page +#define mfn_to_page(mfn) __mfn_to_page(mfn_x(mfn)) + /* Per-domain grant information. */ struct grant_table { /* @@ -167,7 +173,7 @@ struct gnttab_unmap_common { /* Shared state beteen *_unmap and *_unmap_complete */ uint16_t done; -unsigned long frame; +mfn_t frame; struct domain *rd; grant_ref_t ref; }; @@ -266,7 +272,7 @@ struct active_grant_entry { grant.*/ grant_ref_t trans_gref; struct domain *trans_domain; -unsigned long frame; /* Frame being granted. */ +mfn_t frame; /* Frame being granted. */ #ifndef NDEBUG gfn_t gfn;/* Guest's idea of the frame being granted. */ #endif @@ -371,14 +377,14 @@ static inline unsigned int grant_to_status_frames(unsigned int grant_frames) If rc == GNTST_okay, *page contains the page struct with a ref taken. Caller must do put_page(*page). If any error, *page = NULL, *frame = INVALID_MFN, no ref taken. */ -static int get_paged_frame(unsigned long gfn, unsigned long *frame, +static int get_paged_frame(unsigned long gfn, mfn_t *frame, struct page_info **page, bool readonly, struct domain *rd) { int rc = GNTST_okay; p2m_type_t p2mt; -*frame = mfn_x(INVALID_MFN); +*frame = INVALID_MFN; *page = get_page_from_gfn(rd, gfn, &p2mt, readonly ? P2M_ALLOC : P2M_UNSHARE); if ( !*page ) @@ -823,7 +829,7 @@ static int _set_status(unsigned gt_version, static struct active_grant_entry *grant_map_exists(const struct domain *ld, struct grant_table *rgt, - unsigned long mfn, + mfn_t mfn, grant_ref_t *cur_ref) { grant_ref_t ref, max_iter; @@ -842,7 +848,8 @@ static struct active_grant_entry *grant_map_exists(const struct domain *ld, { struct active_grant_entry *act = active_entry_acquire(rgt, ref); -if ( act->pin && act->domid == ld->domain_id && act->frame == mfn ) +if ( act->pin && act->domid == ld->domain_id && + mfn_eq(act->frame, mfn) ) return act; active_entry_release(act); } @@ -859,7 +866,7 @@ static struct active_grant_entry *grant_map_exists(const struct domain *ld, #define MAPKIND_READ 1 #define MAPKIND_WRITE 2 static unsigned int mapkind( -struct grant_table *lgt, const struct domain *rd, unsigned long mfn) +struct grant_table *lgt, const struct domain *rd, mfn_t mfn) { struct grant_mapping *map; grant_handle_t handle, limit = lgt->maptrack_limit; @@ -884,7 +891,7 @@ static unsigned int mapkind( if ( !(map->flags & (GNTMAP_device_map|GNTMAP_host_map)) || map->domid != rd->domain_id ) continue; -if ( _active_entry(rd->grant_table, map->ref).frame == mfn ) +if ( mfn_eq(_active_entry(rd->grant_table, map->ref).frame, mfn) ) kind |= map->flags & GNTMAP_readonly ? MAPKIND_READ : MAPKIND_WRITE; } @@ -907,7 +914,7 @@ map_grant_ref( struct grant_tab
[Xen-devel] [PATCH v5 13/16] xen/grant: Switch {create, replace}_grant_p2m_mapping to typesafe MFN
From: Julien Grall The current prototype is slightly confusing because it takes a guest physical address and a machine physical frame (not address!). Switching to MFN will improve safety and reduce the chance to mistakenly invert the 2 parameters. Signed-off-by: Julien grall Reviewed-by: Wei Liu Reviewed-by: Jan Beulich --- Cc: Stefano Stabellini Cc: Julien Grall Cc: Andrew Cooper Cc: George Dunlap Cc: Ian Jackson Cc: Jan Beulich Cc: Konrad Rzeszutek Wilk Cc: Tim Deegan Cc: Wei Liu Changes in v5: - Add Wei's and Jan's reviewed-by Changes in v4: - Patch added --- xen/arch/arm/mm.c | 10 +- xen/arch/x86/hvm/grant_table.c| 14 +++--- xen/arch/x86/pv/grant_table.c | 10 +- xen/common/grant_table.c | 8 xen/include/asm-arm/grant_table.h | 9 - xen/include/asm-x86/grant_table.h | 4 ++-- xen/include/asm-x86/hvm/grant_table.h | 8 xen/include/asm-x86/pv/grant_table.h | 8 8 files changed, 35 insertions(+), 36 deletions(-) diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index f17907ace8..4268dd5c2d 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -1423,7 +1423,7 @@ void gnttab_mark_dirty(struct domain *d, unsigned long l) } } -int create_grant_host_mapping(unsigned long addr, unsigned long frame, +int create_grant_host_mapping(unsigned long addr, mfn_t frame, unsigned int flags, unsigned int cache_flags) { int rc; @@ -1436,7 +1436,7 @@ int create_grant_host_mapping(unsigned long addr, unsigned long frame, t = p2m_grant_map_ro; rc = guest_physmap_add_entry(current->domain, gaddr_to_gfn(addr), - _mfn(frame), 0, t); + frame, 0, t); if ( rc ) return GNTST_general_error; @@ -1444,8 +1444,8 @@ int create_grant_host_mapping(unsigned long addr, unsigned long frame, return GNTST_okay; } -int replace_grant_host_mapping(unsigned long addr, unsigned long mfn, -unsigned long new_addr, unsigned int flags) +int replace_grant_host_mapping(unsigned long addr, mfn_t mfn, + unsigned long new_addr, unsigned int flags) { gfn_t gfn = gaddr_to_gfn(addr); struct domain *d = current->domain; @@ -1454,7 +1454,7 @@ int replace_grant_host_mapping(unsigned long addr, unsigned long mfn, if ( new_addr != 0 || (flags & GNTMAP_contains_pte) ) return GNTST_general_error; -rc = guest_physmap_remove_page(d, gfn, _mfn(mfn), 0); +rc = guest_physmap_remove_page(d, gfn, mfn, 0); return rc ? GNTST_general_error : GNTST_okay; } diff --git a/xen/arch/x86/hvm/grant_table.c b/xen/arch/x86/hvm/grant_table.c index 9ca9fe0425..ecd7d078ab 100644 --- a/xen/arch/x86/hvm/grant_table.c +++ b/xen/arch/x86/hvm/grant_table.c @@ -25,7 +25,7 @@ #include -int create_grant_p2m_mapping(uint64_t addr, unsigned long frame, +int create_grant_p2m_mapping(uint64_t addr, mfn_t frame, unsigned int flags, unsigned int cache_flags) { @@ -41,14 +41,14 @@ int create_grant_p2m_mapping(uint64_t addr, unsigned long frame, p2mt = p2m_grant_map_rw; rc = guest_physmap_add_entry(current->domain, _gfn(addr >> PAGE_SHIFT), - _mfn(frame), PAGE_ORDER_4K, p2mt); + frame, PAGE_ORDER_4K, p2mt); if ( rc ) return GNTST_general_error; else return GNTST_okay; } -int replace_grant_p2m_mapping(uint64_t addr, unsigned long frame, +int replace_grant_p2m_mapping(uint64_t addr, mfn_t frame, uint64_t new_addr, unsigned int flags) { unsigned long gfn = (unsigned long)(addr >> PAGE_SHIFT); @@ -60,15 +60,15 @@ int replace_grant_p2m_mapping(uint64_t addr, unsigned long frame, return GNTST_general_error; old_mfn = get_gfn(d, gfn, &type); -if ( !p2m_is_grant(type) || mfn_x(old_mfn) != frame ) +if ( !p2m_is_grant(type) || !mfn_eq(old_mfn, frame) ) { put_gfn(d, gfn); gdprintk(XENLOG_WARNING, - "old mapping invalid (type %d, mfn %" PRI_mfn ", frame %lx)\n", - type, mfn_x(old_mfn), frame); + "old mapping invalid (type %d, mfn %" PRI_mfn ", frame %"PRI_mfn")\n", + type, mfn_x(old_mfn), mfn_x(frame)); return GNTST_general_error; } -if ( guest_physmap_remove_page(d, _gfn(gfn), _mfn(frame), PAGE_ORDER_4K) ) +if ( guest_physmap_remove_page(d, _gfn(gfn), frame, PAGE_ORDER_4K) ) { put_gfn(d, gfn); return GNTST_general_error; diff --git a/xen/arch/x86/pv/grant_table.c b/xen/arch/x86/pv/grant_table.c index 4dbc550366..458085e1b6 100644 --- a/xen/arch/x86/pv/grant_table.c +++ b/xen/arch/x86/pv/grant_table.c @@ -50,7 +50,7
[Xen-devel] [PATCH v5 03/16] xen/arm: mm: Use gaddr_to_gfn rather than _gfn(paddr_to_pfn(...))
From: Julien Grall The construction _gfn(paddr_to_pfn(...)) can be simplified by using gaddr_to_gfn. Signed-off-by: Julien Grall --- Cc: Stefano Stabellini Changes in v4: - Patch added --- xen/arch/arm/mm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 3c328e2df5..9b77ab5f33 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -1436,7 +1436,7 @@ int create_grant_host_mapping(unsigned long addr, unsigned long frame, if ( flags & GNTMAP_readonly ) t = p2m_grant_map_ro; -rc = guest_physmap_add_entry(current->domain, _gfn(addr >> PAGE_SHIFT), +rc = guest_physmap_add_entry(current->domain, gaddr_to_gfn(addr), _mfn(frame), 0, t); if ( rc ) @@ -1448,7 +1448,7 @@ int create_grant_host_mapping(unsigned long addr, unsigned long frame, int replace_grant_host_mapping(unsigned long addr, unsigned long mfn, unsigned long new_addr, unsigned int flags) { -gfn_t gfn = _gfn(addr >> PAGE_SHIFT); +gfn_t gfn = gaddr_to_gfn(addr); struct domain *d = current->domain; int rc; -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 4/6] xen/arm: gic: Split the field state in gic_lr in 2 fields active and pending
On 03/14/2018 06:09 PM, Andre Przywara wrote: On 09/03/18 16:35, julien.gr...@arm.com wrote: diff --git a/xen/include/asm-arm/gic_v3_defs.h b/xen/include/asm-arm/gic_v3_defs.h index ccb72cf0f1..817bb0d5c7 100644 --- a/xen/include/asm-arm/gic_v3_defs.h +++ b/xen/include/asm-arm/gic_v3_defs.h @@ -171,6 +171,8 @@ #define ICH_LR_PHYSICAL_SHIFT32 #define ICH_LR_STATE_MASK0x3 #define ICH_LR_STATE_SHIFT 62 +#define ICH_LR_STATE_PENDING (1UL << 62) +#define ICH_LR_STATE_ACTIVE (1UL << 63) Should that be 1ULL, just in case we ever get 32-bit support for GICv3? Yes, good point. I will fix that. Regardless of that: Reviewed-by: Andre Przywara Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v5 04/16] xen/arm: mm: Remove unused M2P code
From: Julien Grall Arm does not have an M2P and very unlikely to get one in the future, therefore don't keep defines that are not necessary in the common code. At the same time move the remaining M2P define just above just above set_gpfn_from_mfn to keep all the dummy helpers for M2P together. Signed-off-by: Julien Grall --- Cc: Stefano Stabellini Changes in v4: - Patch added. --- xen/include/asm-arm/mm.h | 25 - 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h index 4d5563b0ce..c03f4ad674 100644 --- a/xen/include/asm-arm/mm.h +++ b/xen/include/asm-arm/mm.h @@ -320,33 +320,16 @@ static inline void *page_to_virt(const struct page_info *pg) struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va, unsigned long flags); -/* - * The MPT (machine->physical mapping table) is an array of word-sized - * values, indexed on machine frame number. It is expected that guest OSes - * will use it to store a "physical" frame number to give the appearance of - * contiguous (or near contiguous) physical memory. - */ -#undef machine_to_phys_mapping -#define machine_to_phys_mapping ((unsigned long *)RDWR_MPT_VIRT_START) -#define INVALID_M2P_ENTRY(~0UL) -#define VALID_M2P(_e)(!((_e) & (1UL<<(BITS_PER_LONG-1 -#define SHARED_M2P_ENTRY (~0UL - 1UL) -#define SHARED_M2P(_e) ((_e) == SHARED_M2P_ENTRY) - -#define _set_gpfn_from_mfn(mfn, pfn) ({\ -struct domain *d = page_get_owner(__mfn_to_page(mfn)); \ -if(d && (d == dom_cow))\ -machine_to_phys_mapping[(mfn)] = SHARED_M2P_ENTRY; \ -else \ -machine_to_phys_mapping[(mfn)] = (pfn);\ -}) - static inline void put_gfn(struct domain *d, unsigned long gfn) {} static inline int relinquish_shared_pages(struct domain *d) { return 0; } +#define INVALID_M2P_ENTRY(~0UL) +#define SHARED_M2P_ENTRY (~0UL - 1UL) +#define SHARED_M2P(_e) ((_e) == SHARED_M2P_ENTRY) + /* Xen always owns P2M on ARM */ #define set_gpfn_from_mfn(mfn, pfn) do { (void) (mfn), (void)(pfn); } while (0) #define mfn_to_gmfn(_d, mfn) (mfn) -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v5 11/16] xen/mm: Switch some of page_alloc.c to typesafe MFN
From: Julien Grall No functional change intended. Signed-off-by: Julien Grall Reviewed-by: Wei Liu --- Cc: Andrew Cooper Cc: George Dunlap Cc: Ian Jackson Cc: Jan Beulich Cc: Konrad Rzeszutek Wilk Cc: Stefano Stabellini Cc: Tim Deegan Cc: Wei Liu Cc: Julien Grall Wei, I have kept the reviewed-by because the changes were minor. Let's me know if you want me to drop it. Changes in v5: - Add Wei's reviewed-by - Fix coding style (space before and after '+') - Rework the commit title as page_alloc.c was not fully converted to typesafe MFN. Changes in v4: - Patch added --- xen/common/page_alloc.c| 64 ++ xen/include/asm-arm/numa.h | 8 +++--- 2 files changed, 41 insertions(+), 31 deletions(-) diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index 4de8988bea..6e50fb2621 100644 --- a/xen/common/page_alloc.c +++ b/xen/common/page_alloc.c @@ -151,6 +151,12 @@ #define p2m_pod_offline_or_broken_replace(pg) BUG_ON(pg != NULL) #endif +/* Override macros from asm/page.h to make them work with mfn_t */ +#undef page_to_mfn +#define page_to_mfn(pg) _mfn(__page_to_mfn(pg)) +#undef mfn_to_page +#define mfn_to_page(mfn) __mfn_to_page(mfn_x(mfn)) + /* * Comma-separated list of hexadecimal page numbers containing bad bytes. * e.g. 'badpage=0x3f45,0x8a321'. @@ -197,7 +203,7 @@ PAGE_LIST_HEAD(page_broken_list); * first_valid_mfn is exported because it is use in ARM specific NUMA * helpers. See comment in asm-arm/numa.h. */ -unsigned long first_valid_mfn = ~0UL; +mfn_t first_valid_mfn = INVALID_MFN_INITIALIZER; static struct bootmem_region { unsigned long s, e; /* MFNs @s through @e-1 inclusive are free */ @@ -283,7 +289,7 @@ void __init init_boot_pages(paddr_t ps, paddr_t pe) if ( pe <= ps ) return; -first_valid_mfn = min_t(unsigned long, ps >> PAGE_SHIFT, first_valid_mfn); +first_valid_mfn = mfn_min(maddr_to_mfn(ps), first_valid_mfn); bootmem_region_add(ps >> PAGE_SHIFT, pe >> PAGE_SHIFT); @@ -397,7 +403,7 @@ mfn_t __init alloc_boot_pages(unsigned long nr_pfns, unsigned long pfn_align) #define bits_to_zone(b) (((b) < (PAGE_SHIFT + 1)) ? 1 : ((b) - PAGE_SHIFT)) #define page_to_zone(pg) (is_xen_heap_page(pg) ? MEMZONE_XEN : \ - (flsl(page_to_mfn(pg)) ? : 1)) + (flsl(mfn_x(page_to_mfn(pg))) ? : 1)) typedef struct page_list_head heap_by_zone_and_order_t[NR_ZONES][MAX_ORDER+1]; static heap_by_zone_and_order_t *_heap[MAX_NUMNODES]; @@ -729,7 +735,7 @@ static void page_list_add_scrub(struct page_info *pg, unsigned int node, static void poison_one_page(struct page_info *pg) { #ifdef CONFIG_SCRUB_DEBUG -mfn_t mfn = _mfn(page_to_mfn(pg)); +mfn_t mfn = page_to_mfn(pg); uint64_t *ptr; if ( !scrub_debug ) @@ -744,7 +750,7 @@ static void poison_one_page(struct page_info *pg) static void check_one_page(struct page_info *pg) { #ifdef CONFIG_SCRUB_DEBUG -mfn_t mfn = _mfn(page_to_mfn(pg)); +mfn_t mfn = page_to_mfn(pg); const uint64_t *ptr; unsigned int i; @@ -992,7 +998,8 @@ static struct page_info *alloc_heap_pages( /* Ensure cache and RAM are consistent for platforms where the * guest can control its own visibility of/through the cache. */ -flush_page_to_ram(page_to_mfn(&pg[i]), !(memflags & MEMF_no_icache_flush)); +flush_page_to_ram(mfn_x(page_to_mfn(&pg[i])), + !(memflags & MEMF_no_icache_flush)); } spin_unlock(&heap_lock); @@ -1344,7 +1351,8 @@ bool scrub_free_pages(void) static void free_heap_pages( struct page_info *pg, unsigned int order, bool need_scrub) { -unsigned long mask, mfn = page_to_mfn(pg); +unsigned long mask; +mfn_t mfn = page_to_mfn(pg); unsigned int i, node = phys_to_nid(page_to_maddr(pg)), tainted = 0; unsigned int zone = page_to_zone(pg); @@ -1381,7 +1389,7 @@ static void free_heap_pages( /* This page is not a guest frame any more. */ page_set_owner(&pg[i], NULL); /* set_gpfn_from_mfn snoops pg owner */ -set_gpfn_from_mfn(mfn + i, INVALID_M2P_ENTRY); +set_gpfn_from_mfn(mfn_x(mfn) + i, INVALID_M2P_ENTRY); if ( need_scrub ) { @@ -1409,12 +1417,12 @@ static void free_heap_pages( { mask = 1UL << order; -if ( (page_to_mfn(pg) & mask) ) +if ( (mfn_x(page_to_mfn(pg)) & mask) ) { struct page_info *predecessor = pg - mask; /* Merge with predecessor block? */ -if ( !mfn_valid(_mfn(page_to_mfn(predecessor))) || +if ( !mfn_valid(page_to_mfn(predecessor)) || !page_state_is(predecessor, free) || (PFN_ORDER(predecessor) != order) || (phys_to_nid(page_to_maddr(predecessor)) != node) ) @@ -1437,7 +1445,7 @@ static void free_heap_pages(
[Xen-devel] [PATCH v5 09/16] xen/pdx: Introduce helper to convert MFN <-> PDX
From: Julien Grall This will avoid use of pfn_to_pdx(mfn_x(mfn)) over the code base. Signed-off-by: Julien Grall Reviewed-by: Wei Liu --- Cc: Andrew Cooper Cc: George Dunlap Cc: Ian Jackson Cc: Jan Beulich Cc: Konrad Rzeszutek Wilk Cc: Stefano Stabellini Cc: Tim Deegan Cc: Wei Liu Changes in v5: - Add Wei's reviewed-by Changes in v4: - Patch added --- xen/include/xen/pdx.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/xen/include/xen/pdx.h b/xen/include/xen/pdx.h index 4c56645c4c..a151aac1a2 100644 --- a/xen/include/xen/pdx.h +++ b/xen/include/xen/pdx.h @@ -35,6 +35,9 @@ static inline unsigned long pdx_to_pfn(unsigned long pdx) ((pdx << pfn_pdx_hole_shift) & pfn_top_mask); } +#define mfn_to_pdx(mfn) pfn_to_pdx(mfn_x(mfn)) +#define pdx_to_mfn(pdx) _mfn(pdx_to_pfn(pdx)) + extern void pfn_pdx_hole_setup(unsigned long); #endif /* HAS_PDX */ -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v5 00/16] xen: Convert page_to_mfn and mfn_to_page to use typesafe MFN
From: Julien Grall Hi all, This is v5 of the series for all the changes see in each patch. To avoid the last patch (#16) to be a huge patch some files are converted to use typesafe upfront. I have tried my best to push _mfn/mfn_x as down as possible in the callers. Some of them was not feasible without major rework, so I left them aside for now. Contribution to switch Xen code base to MFN typesafe are more than welcomed. Note that changes have only been build test it on x86 so far. Cheers, Cc: Andrew Cooper Cc: Boris Ostrovsky Cc: Gang Wei Cc: George Dunlap Cc: George Dunlap Cc: Ian Jackson Cc: Jan Beulich Cc: Julien Grall Cc: Jun Nakajima Cc: Kevin Tian Cc: Konrad Rzeszutek Wilk Cc: Paul Durrant Cc: Razvan Cojocaru Cc: Shane Wang Cc: Stefano Stabellini Cc: Suravee Suthikulpanit Cc: Tamas K Lengyel Cc: Tim Deegan Cc: Wei Liu Julien Grall (15): xen/arm: setup: use maddr_to_mfn rather than _mfn(paddr_to_pfn(...)) xen/arm: mm: Use gaddr_to_gfn rather than _gfn(paddr_to_pfn(...)) xen/arm: mm: Remove unused M2P code xen/arm: mm: Remove unused relinquish_shared_pages xen/x86: Remove unused override of page_to_mfn/mfn_to_page xen/x86: mm: Switch x86/mm.c to use typesafe for virt_to_mfn xen/mm: Drop the parameter mfn from populate_pt_range xen/pdx: Introduce helper to convert MFN <-> PDX xen/mm: Switch map_pages_to_xen to use MFN typesafe xen/mm: Switch some of page_alloc.c to typesafe MFN xen/mm: Switch common/memory.c to use typesafe MFN xen/grant: Switch {create, replace}_grant_p2m_mapping to typesafe MFN xen/grant: Switch common/grant_table.c to use typesafe MFN xen/x86: Switch mfn_to_page in x86_64/mm.c to use typesafe MFN xen: Convert page_to_mfn and mfn_to_page to use typesafe MFN Wei Liu (1): x86/mm: skip incrementing mfn if it is not a valid mfn xen/arch/arm/domain_build.c | 2 - xen/arch/arm/kernel.c | 2 +- xen/arch/arm/mem_access.c | 2 +- xen/arch/arm/mm.c | 33 xen/arch/arm/p2m.c | 10 +-- xen/arch/arm/setup.c| 4 +- xen/arch/x86/cpu/vpmu.c | 4 +- xen/arch/x86/domain.c | 21 ++--- xen/arch/x86/domain_page.c | 6 +- xen/arch/x86/hvm/dm.c | 2 +- xen/arch/x86/hvm/dom0_build.c | 6 +- xen/arch/x86/hvm/emulate.c | 6 +- xen/arch/x86/hvm/grant_table.c | 14 ++-- xen/arch/x86/hvm/hvm.c | 12 +-- xen/arch/x86/hvm/ioreq.c| 4 +- xen/arch/x86/hvm/stdvga.c | 2 +- xen/arch/x86/hvm/svm/svm.c | 4 +- xen/arch/x86/hvm/viridian.c | 6 +- xen/arch/x86/hvm/vmx/vmcs.c | 2 +- xen/arch/x86/hvm/vmx/vmx.c | 10 +-- xen/arch/x86/hvm/vmx/vvmx.c | 6 +- xen/arch/x86/mm.c | 75 +- xen/arch/x86/mm/guest_walk.c| 6 +- xen/arch/x86/mm/hap/guest_walk.c| 2 +- xen/arch/x86/mm/hap/hap.c | 6 -- xen/arch/x86/mm/hap/nested_ept.c| 2 +- xen/arch/x86/mm/hap/nested_hap.c| 3 - xen/arch/x86/mm/mem_sharing.c | 5 -- xen/arch/x86/mm/p2m-ept.c | 8 +- xen/arch/x86/mm/p2m-pod.c | 6 -- xen/arch/x86/mm/p2m-pt.c| 6 -- xen/arch/x86/mm/p2m.c | 6 -- xen/arch/x86/mm/paging.c| 6 -- xen/arch/x86/mm/shadow/private.h| 16 +--- xen/arch/x86/numa.c | 2 +- xen/arch/x86/physdev.c | 2 +- xen/arch/x86/pv/callback.c | 6 -- xen/arch/x86/pv/descriptor-tables.c | 6 -- xen/arch/x86/pv/dom0_build.c| 14 ++-- xen/arch/x86/pv/domain.c| 6 -- xen/arch/x86/pv/emul-gate-op.c | 6 -- xen/arch/x86/pv/emul-priv-op.c | 10 --- xen/arch/x86/pv/grant_table.c | 16 ++-- xen/arch/x86/pv/iret.c | 6 -- xen/arch/x86/pv/mm.c| 6 -- xen/arch/x86/pv/ro-page-fault.c | 6 -- xen/arch/x86/pv/shim.c | 4 +- xen/arch/x86/pv/traps.c | 6 -- xen/arch/x86/setup.c| 20 ++--- xen/arch/x86/smpboot.c | 8 +- xen/arch/x86/tboot.c| 15 ++-- xen/arch/x86/traps.c| 4 +- xen/arch/x86/x86_64/mm.c| 67 xen/arch/x86/x86_64/mmconfig_64.c | 6 +- xen/common/domain.c | 4 +- xen/common/efi/boot.c | 2 +- xen/common/grant_table.c| 133 +--- xen/common/kimage.c | 6 -- xen/common/memory.c | 69 ++--- xen/common/page_alloc.c | 58 +++--- xen/common/tmem.c | 2 +-
[Xen-devel] [PATCH v5 01/16] x86/mm: skip incrementing mfn if it is not a valid mfn
From: Wei Liu The function is called to fill in page table entries in populate_pt_range. Skip incrementing mfn if it is invalid. Signed-off-by: Wei Liu --- Changes in v5: - Patch added --- xen/arch/x86/mm.c | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 9b559448a7..5f5577c7c2 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -4731,7 +4731,8 @@ int map_pages_to_xen( } virt+= 1UL << L3_PAGETABLE_SHIFT; -mfn += 1UL << (L3_PAGETABLE_SHIFT - PAGE_SHIFT); +if ( !mfn_eq(_mfn(mfn), INVALID_MFN) ) +mfn += 1UL << (L3_PAGETABLE_SHIFT - PAGE_SHIFT); nr_mfns -= 1UL << (L3_PAGETABLE_SHIFT - PAGE_SHIFT); continue; } @@ -4756,7 +4757,8 @@ int map_pages_to_xen( if ( i > nr_mfns ) i = nr_mfns; virt+= i << PAGE_SHIFT; -mfn += i; +if ( !mfn_eq(_mfn(mfn), INVALID_MFN) ) +mfn += i; nr_mfns -= i; continue; } @@ -4824,7 +4826,8 @@ int map_pages_to_xen( } virt+= 1UL << L2_PAGETABLE_SHIFT; -mfn += 1UL << PAGETABLE_ORDER; +if ( !mfn_eq(_mfn(mfn), INVALID_MFN) ) +mfn += 1UL << PAGETABLE_ORDER; nr_mfns -= 1UL << PAGETABLE_ORDER; } else @@ -4853,7 +4856,8 @@ int map_pages_to_xen( if ( i > nr_mfns ) i = nr_mfns; virt+= i << L1_PAGETABLE_SHIFT; -mfn += i; +if ( !mfn_eq(_mfn(mfn), INVALID_MFN) ) +mfn += i; nr_mfns -= i; goto check_l3; } @@ -4898,7 +4902,8 @@ int map_pages_to_xen( } virt+= 1UL << L1_PAGETABLE_SHIFT; -mfn += 1UL; +if ( !mfn_eq(_mfn(mfn), INVALID_MFN) ) +mfn += 1UL; nr_mfns -= 1UL; if ( (flags == PAGE_HYPERVISOR) && -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 6/6] ARM: GIC: extend LR read/write functions to cover EOI and source
Hi, On 09/03/18 16:35, julien.gr...@arm.com wrote: > From: Andre Przywara I think this is quite different from what I ever wrote, so please drop my authorship here. > So far our LR read/write functions do not handle the EOI bit and the > source CPUID bits in an LR, because the current VGIC implementation does > not use them. > Extend the gic_lr data structure to hold these bits of information by > using a union to differentiate field used depending on whether the vIRQ > has a corresponding pIRQ. As mentioned before, I am not sure this is really necessary. The idea of struct gic_lr is to provide a hardware agnostic view of an LR. So the actual read_lr/write_lr function take care of reading/populating pINTID, iff the HW bit is set (as done in your patch 5/6). Given that, I don't think we need to change the current code in this respect, as this is just a small internal interface. But then again I don't care enough, so if that makes you happy > Note that source is not covered by GICv3 LR. I was thinking the same, but Marc pointed me to that inconspicuous note on page 8-299 in ARM IHI 0069D (at the end of "vINTID", chapter 8.4.6). > This allows the new VGIC to use this information. > > Signed-off-by: Andre Przywara > Signed-off-by: Julien Grall > --- > xen/arch/arm/gic-v2.c | 22 +++--- > xen/arch/arm/gic-v3.c | 11 +-- > xen/include/asm-arm/gic.h | 16 ++-- > 3 files changed, 42 insertions(+), 7 deletions(-) > > diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c > index daf8c61258..69f8d6044a 100644 > --- a/xen/arch/arm/gic-v2.c > +++ b/xen/arch/arm/gic-v2.c > @@ -474,8 +474,17 @@ static void gicv2_read_lr(int lr, struct gic_lr *lr_reg) > > if ( lr_reg->hw_status ) > { > -lr_reg->pirq = lrv >> GICH_V2_LR_PHYSICAL_SHIFT; > -lr_reg->pirq &= GICH_V2_LR_PHYSICAL_MASK; > +lr_reg->h.pirq = lrv >> GICH_V2_LR_PHYSICAL_SHIFT; > +lr_reg->h.pirq &= GICH_V2_LR_PHYSICAL_MASK; > +} > +else > +{ > +lr_reg->v.eoi = (lrv & GICH_V2_LR_MAINTENANCE_IRQ) == > GICH_V2_LR_MAINTENANCE_IRQ; I think you can drop the " == GICH_V2_LR_MAINTENANCE_IRQ", as .eoi is a bool. Avoids the linebreak. > +/* > + * This is only valid for SGI, but it does not matter to always > + * read it as it should be 0 by default. > + */ > +lr_reg->v.source = (lrv >> GICH_V2_LR_CPUID_SHIFT) & > GICH_V2_LR_CPUID_MASK; > } > } > > @@ -496,7 +505,14 @@ static void gicv2_write_lr(int lr, const struct gic_lr > *lr_reg) > if ( lr_reg->hw_status ) > { > lrv |= GICH_V2_LR_HW; > -lrv |= lr_reg->pirq << GICH_V2_LR_PHYSICAL_SHIFT; > +lrv |= lr_reg->h.pirq << GICH_V2_LR_PHYSICAL_SHIFT; > +} > +else > +{ > +if ( lr_reg->v.eoi ) > +lrv |= GICH_V2_LR_MAINTENANCE_IRQ; > +if ( lr_reg->virq < NR_GIC_SGI ) > +lrv |= (uint32_t)lr_reg->v.source << GICH_V2_LR_CPUID_SHIFT; > } > > writel_gich(lrv, GICH_LR + lr * 4); > diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c > index f73d386df1..a855069111 100644 > --- a/xen/arch/arm/gic-v3.c > +++ b/xen/arch/arm/gic-v3.c > @@ -1014,7 +1014,9 @@ static void gicv3_read_lr(int lr, struct gic_lr *lr_reg) > lr_reg->hw_status = (lrv & ICH_LR_HW) == ICH_LR_HW; > > if ( lr_reg->hw_status ) > -lr_reg->pirq = (lrv >> ICH_LR_PHYSICAL_SHIFT) & ICH_LR_PHYSICAL_MASK; > +lr_reg->h.pirq = (lrv >> ICH_LR_PHYSICAL_SHIFT) & > ICH_LR_PHYSICAL_MASK; > +else > +lr_reg->v.eoi = (lrv & ICH_LR_MAINTENANCE_IRQ) == > ICH_LR_MAINTENANCE_IRQ; Same here. > } > > static void gicv3_write_lr(int lr_reg, const struct gic_lr *lr) > @@ -1033,7 +1035,12 @@ static void gicv3_write_lr(int lr_reg, const struct > gic_lr *lr) > if ( lr->hw_status ) > { > lrv |= ICH_LR_HW; > -lrv |= (uint64_t)lr->pirq << ICH_LR_PHYSICAL_SHIFT; > +lrv |= (uint64_t)lr->h.pirq << ICH_LR_PHYSICAL_SHIFT; > +} > +else > +{ > +if ( lr->v.eoi ) > +lrv |= ICH_LR_MAINTENANCE_IRQ; > } > > /* > diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h > index 545901b120..4cf5bb385d 100644 > --- a/xen/include/asm-arm/gic.h > +++ b/xen/include/asm-arm/gic.h > @@ -204,14 +204,26 @@ union gic_state_data { > * The LR register format is different for GIC HW version > */ > struct gic_lr { > - /* Physical IRQ -> Only set when hw_status is set. */ > - uint32_t pirq; > /* Virtual IRQ */ > uint32_t virq; > uint8_t priority; > bool active; > bool pending; > bool hw_status; > + union > + { > + /* Only filled when there are a corresponding pIRQ (hw_state = true) > */ > + struct > + { > + uint32_t pirq; > + } h; > + /* Only filled when there are no corresponding pIRQ (hw_state = > false) */ > + struct > + { > +
[Xen-devel] [PATCH v5 08/16] xen/mm: Drop the parameter mfn from populate_pt_range
From: Julien Grall The function populate_pt_range is used to populate in advance the page-table but it will not do the actual mapping. So passing the MFN in parameter is pointless. Note that the only caller pass 0... At the same time replace 0 by INVALID_MFNs. While this does not matter as the entry will marked as not valid and populated, INVALID_MFN helps the reader to know the MFN is invalid. Signed-off-by: Julien Grall -- Cc: Stefano Stabellini Cc: Julien Grall Cc: Andrew Cooper Cc: George Dunlap Cc: Ian Jackson Cc: Jan Beulich Cc: Konrad Rzeszutek Wilk Cc: Tim Deegan Cc: Wei Liu Changes in v5: - Update the commit message to explain why 0 -> INVALID_MFN. Changes in v4: - Patch added. --- xen/arch/arm/mm.c| 5 ++--- xen/arch/x86/mm.c| 5 ++--- xen/common/vmap.c| 2 +- xen/include/xen/mm.h | 3 +-- 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 9b77ab5f33..97dcdd5d50 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -1072,10 +1072,9 @@ int map_pages_to_xen(unsigned long virt, return create_xen_entries(INSERT, virt, _mfn(mfn), nr_mfns, flags); } -int populate_pt_range(unsigned long virt, unsigned long mfn, - unsigned long nr_mfns) +int populate_pt_range(unsigned long virt, unsigned long nr_mfns) { -return create_xen_entries(RESERVE, virt, _mfn(mfn), nr_mfns, 0); +return create_xen_entries(RESERVE, virt, INVALID_MFN, nr_mfns, 0); } int destroy_xen_mappings(unsigned long v, unsigned long e) diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index ab10f552ea..5e3e870260 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -5013,10 +5013,9 @@ int map_pages_to_xen( return 0; } -int populate_pt_range(unsigned long virt, unsigned long mfn, - unsigned long nr_mfns) +int populate_pt_range(unsigned long virt, unsigned long nr_mfns) { -return map_pages_to_xen(virt, mfn, nr_mfns, MAP_SMALL_PAGES); +return map_pages_to_xen(virt, mfn_x(INVALID_MFN), nr_mfns, MAP_SMALL_PAGES); } /* diff --git a/xen/common/vmap.c b/xen/common/vmap.c index 0b23f8fb97..11785ffb0a 100644 --- a/xen/common/vmap.c +++ b/xen/common/vmap.c @@ -42,7 +42,7 @@ void __init vm_init_type(enum vmap_region type, void *start, void *end) bitmap_fill(vm_bitmap(type), vm_low[type]); /* Populate page tables for the bitmap if necessary. */ -populate_pt_range(va, 0, vm_low[type] - nr); +populate_pt_range(va, vm_low[type] - nr); } static void *vm_alloc(unsigned int nr, unsigned int align, diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h index 0e0e5112c6..f2c6738ad2 100644 --- a/xen/include/xen/mm.h +++ b/xen/include/xen/mm.h @@ -175,8 +175,7 @@ int destroy_xen_mappings(unsigned long v, unsigned long e); * Create only non-leaf page table entries for the * page range in Xen virtual address space. */ -int populate_pt_range(unsigned long virt, unsigned long mfn, - unsigned long nr_mfns); +int populate_pt_range(unsigned long virt, unsigned long nr_mfns); /* Claim handling */ unsigned long domain_adjust_tot_pages(struct domain *d, long pages); int domain_set_outstanding_pages(struct domain *d, unsigned long pages); -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v5 02/16] xen/arm: setup: use maddr_to_mfn rather than _mfn(paddr_to_pfn(...))
From: Julien Grall The construction _mfn(paddr_to_pfn(...)) can be simplified by using maddr_to_mfn. Signed-off-by: Julien Grall --- Cc: Stefano Stabellini Changes in v4: - Patch added --- xen/arch/arm/setup.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index 462736633b..9e1450b7d4 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -268,8 +268,8 @@ void __init discard_initial_modules(void) if ( mi->module[i].kind == BOOTMOD_XEN ) continue; -if ( !mfn_valid(_mfn(paddr_to_pfn(s))) || - !mfn_valid(_mfn(paddr_to_pfn(e +if ( !mfn_valid(maddr_to_mfn(s)) || + !mfn_valid(maddr_to_mfn(e)) ) continue; dt_unreserved_regions(s, e, init_domheap_pages, 0); -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v5 06/16] xen/x86: Remove unused override of page_to_mfn/mfn_to_page
From: Julien Grall A few files override page_to_mfn/mfn_to_page but actually never use those macros. So drop them. Signed-off-by: Julien Grall Acked-by: George Dunlap Acked-by: Jan Beulich --- Cc: George Dunlap Cc: Jan Beulich Cc: Andrew Cooper Changes in v5: - Add George and Jan's acked-by Changes in v4: - Patch added --- xen/arch/x86/mm/hap/nested_hap.c | 3 --- xen/arch/x86/mm/p2m-pt.c | 6 -- xen/arch/x86/pv/iret.c | 6 -- xen/arch/x86/pv/mm.c | 6 -- xen/arch/x86/pv/traps.c | 6 -- 5 files changed, 27 deletions(-) diff --git a/xen/arch/x86/mm/hap/nested_hap.c b/xen/arch/x86/mm/hap/nested_hap.c index 4603ceced4..d2a07a5c79 100644 --- a/xen/arch/x86/mm/hap/nested_hap.c +++ b/xen/arch/x86/mm/hap/nested_hap.c @@ -70,9 +70,6 @@ // /*NESTED VIRT P2M FUNCTIONS */ // -/* Override macros from asm/page.h to make them work with mfn_t */ -#undef page_to_mfn -#define page_to_mfn(_pg) _mfn(__page_to_mfn(_pg)) void nestedp2m_write_p2m_entry(struct p2m_domain *p2m, unsigned long gfn, diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c index 753124bdcd..b8c5d2ed26 100644 --- a/xen/arch/x86/mm/p2m-pt.c +++ b/xen/arch/x86/mm/p2m-pt.c @@ -39,12 +39,6 @@ #include "mm-locks.h" -/* Override macros from asm/page.h to make them work with mfn_t */ -#undef mfn_to_page -#define mfn_to_page(_m) __mfn_to_page(mfn_x(_m)) -#undef page_to_mfn -#define page_to_mfn(_pg) _mfn(__page_to_mfn(_pg)) - /* * We may store INVALID_MFN in PTEs. We need to clip this to avoid trampling * over higher-order bits (NX, p2m type, IOMMU flags). We seem to not need diff --git a/xen/arch/x86/pv/iret.c b/xen/arch/x86/pv/iret.c index 56aeac360a..ca433a69c4 100644 --- a/xen/arch/x86/pv/iret.c +++ b/xen/arch/x86/pv/iret.c @@ -24,12 +24,6 @@ #include #include -/* Override macros from asm/page.h to make them work with mfn_t */ -#undef mfn_to_page -#define mfn_to_page(mfn) __mfn_to_page(mfn_x(mfn)) -#undef page_to_mfn -#define page_to_mfn(pg) _mfn(__page_to_mfn(pg)) - unsigned long do_iret(void) { struct cpu_user_regs *regs = guest_cpu_user_regs(); diff --git a/xen/arch/x86/pv/mm.c b/xen/arch/x86/pv/mm.c index 8d7a4fd85f..b46fd94c2c 100644 --- a/xen/arch/x86/pv/mm.c +++ b/xen/arch/x86/pv/mm.c @@ -26,12 +26,6 @@ #include "mm.h" -/* Override macros from asm/page.h to make them work with mfn_t */ -#undef mfn_to_page -#define mfn_to_page(mfn) __mfn_to_page(mfn_x(mfn)) -#undef page_to_mfn -#define page_to_mfn(pg) _mfn(__page_to_mfn(pg)) - /* * Get a mapping of a PV guest's l1e for this linear address. The return * pointer should be unmapped using unmap_domain_page(). diff --git a/xen/arch/x86/pv/traps.c b/xen/arch/x86/pv/traps.c index 98549bc1ea..f48db92243 100644 --- a/xen/arch/x86/pv/traps.c +++ b/xen/arch/x86/pv/traps.c @@ -29,12 +29,6 @@ #include #include -/* Override macros from asm/page.h to make them work with mfn_t */ -#undef mfn_to_page -#define mfn_to_page(mfn) __mfn_to_page(mfn_x(mfn)) -#undef page_to_mfn -#define page_to_mfn(pg) _mfn(__page_to_mfn(pg)) - void do_entry_int82(struct cpu_user_regs *regs) { if ( unlikely(untrusted_msi) ) -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 5/6] xen/arm: GIC: Only set pirq in the LR when hw_status is set
Hi, On 09/03/18 16:35, julien.gr...@arm.com wrote: > From: Julien Grall > > The field pirq should only be valid when the virtual interrupt > is associated to a physical interrupt. > > This change will help to extend gic_lr for supporting specific virtual > interrupt field (e.g eoi, source) that clashes with the PIRQ field. Makes some sense, yes. > Signed-off-by: Julien Grall Reviewed-by: Andre Przywara Cheers, Andre. > --- > xen/arch/arm/gic-v2.c | 13 ++--- > xen/arch/arm/gic-v3.c | 10 +++--- > xen/include/asm-arm/gic.h | 2 +- > 3 files changed, 18 insertions(+), 7 deletions(-) > > diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c > index 90d8f652d3..daf8c61258 100644 > --- a/xen/arch/arm/gic-v2.c > +++ b/xen/arch/arm/gic-v2.c > @@ -466,20 +466,24 @@ static void gicv2_read_lr(int lr, struct gic_lr *lr_reg) > uint32_t lrv; > > lrv = readl_gich(GICH_LR + lr * 4); > -lr_reg->pirq = (lrv >> GICH_V2_LR_PHYSICAL_SHIFT) & > GICH_V2_LR_PHYSICAL_MASK; > lr_reg->virq = (lrv >> GICH_V2_LR_VIRTUAL_SHIFT) & > GICH_V2_LR_VIRTUAL_MASK; > lr_reg->priority = (lrv >> GICH_V2_LR_PRIORITY_SHIFT) & > GICH_V2_LR_PRIORITY_MASK; > lr_reg->pending = (lrv & GICH_V2_LR_PENDING) == GICH_V2_LR_PENDING; > lr_reg->active = (lrv & GICH_V2_LR_ACTIVE) == GICH_V2_LR_ACTIVE; > lr_reg->hw_status = (lrv & GICH_V2_LR_HW) == GICH_V2_LR_HW; > + > +if ( lr_reg->hw_status ) > +{ > +lr_reg->pirq = lrv >> GICH_V2_LR_PHYSICAL_SHIFT; > +lr_reg->pirq &= GICH_V2_LR_PHYSICAL_MASK; > +} > } > > static void gicv2_write_lr(int lr, const struct gic_lr *lr_reg) > { > uint32_t lrv = 0; > > -lrv = ( ((lr_reg->pirq & GICH_V2_LR_PHYSICAL_MASK) << > GICH_V2_LR_PHYSICAL_SHIFT) | > - ((lr_reg->virq & GICH_V2_LR_VIRTUAL_MASK) << > GICH_V2_LR_VIRTUAL_SHIFT) | > +lrv = (((lr_reg->virq & GICH_V2_LR_VIRTUAL_MASK) << > GICH_V2_LR_VIRTUAL_SHIFT) | >((uint32_t)(lr_reg->priority & GICH_V2_LR_PRIORITY_MASK) ><< GICH_V2_LR_PRIORITY_SHIFT) ); > > @@ -490,7 +494,10 @@ static void gicv2_write_lr(int lr, const struct gic_lr > *lr_reg) > lrv |= GICH_V2_LR_PENDING; > > if ( lr_reg->hw_status ) > +{ > lrv |= GICH_V2_LR_HW; > +lrv |= lr_reg->pirq << GICH_V2_LR_PHYSICAL_SHIFT; > +} > > writel_gich(lrv, GICH_LR + lr * 4); > } > diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c > index 4dbbf0afd2..f73d386df1 100644 > --- a/xen/arch/arm/gic-v3.c > +++ b/xen/arch/arm/gic-v3.c > @@ -1006,21 +1006,22 @@ static void gicv3_read_lr(int lr, struct gic_lr > *lr_reg) > > lrv = gicv3_ich_read_lr(lr); > > -lr_reg->pirq = (lrv >> ICH_LR_PHYSICAL_SHIFT) & ICH_LR_PHYSICAL_MASK; > lr_reg->virq = (lrv >> ICH_LR_VIRTUAL_SHIFT) & ICH_LR_VIRTUAL_MASK; > > lr_reg->priority = (lrv >> ICH_LR_PRIORITY_SHIFT) & > ICH_LR_PRIORITY_MASK; > lr_reg->pending = (lrv & ICH_LR_STATE_PENDING) == ICH_LR_STATE_PENDING; > lr_reg->active= (lrv & ICH_LR_STATE_ACTIVE) == ICH_LR_STATE_ACTIVE; > lr_reg->hw_status = (lrv & ICH_LR_HW) == ICH_LR_HW; > + > +if ( lr_reg->hw_status ) > +lr_reg->pirq = (lrv >> ICH_LR_PHYSICAL_SHIFT) & ICH_LR_PHYSICAL_MASK; > } > > static void gicv3_write_lr(int lr_reg, const struct gic_lr *lr) > { > uint64_t lrv = 0; > > -lrv = ( ((u64)(lr->pirq & ICH_LR_PHYSICAL_MASK) << > ICH_LR_PHYSICAL_SHIFT)| > -((u64)(lr->virq & ICH_LR_VIRTUAL_MASK) << ICH_LR_VIRTUAL_SHIFT) | > +lrv = ( ((u64)(lr->virq & ICH_LR_VIRTUAL_MASK) << ICH_LR_VIRTUAL_SHIFT) > | > ((u64)(lr->priority & ICH_LR_PRIORITY_MASK) << > ICH_LR_PRIORITY_SHIFT) ); > > if ( lr->active ) > @@ -1030,7 +1031,10 @@ static void gicv3_write_lr(int lr_reg, const struct > gic_lr *lr) > lrv |= ICH_LR_STATE_PENDING; > > if ( lr->hw_status ) > +{ > lrv |= ICH_LR_HW; > +lrv |= (uint64_t)lr->pirq << ICH_LR_PHYSICAL_SHIFT; > +} > > /* > * When the guest is using vGICv3, all the IRQs are Group 1. Group 0 > diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h > index c32861d4fa..545901b120 100644 > --- a/xen/include/asm-arm/gic.h > +++ b/xen/include/asm-arm/gic.h > @@ -204,7 +204,7 @@ union gic_state_data { > * The LR register format is different for GIC HW version > */ > struct gic_lr { > - /* Physical IRQ */ > + /* Physical IRQ -> Only set when hw_status is set. */ > uint32_t pirq; > /* Virtual IRQ */ > uint32_t virq; > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 4/6] xen/arm: gic: Split the field state in gic_lr in 2 fields active and pending
Hi, On 09/03/18 16:35, julien.gr...@arm.com wrote: > From: Julien Grall > > Mostly making the code nicer to read. > > Signed-off-by: Julien Grall > --- > xen/arch/arm/gic-v2.c | 15 +++ > xen/arch/arm/gic-v3.c | 12 +--- > xen/arch/arm/gic-vgic.c | 6 +++--- > xen/include/asm-arm/gic.h | 3 ++- > xen/include/asm-arm/gic_v3_defs.h | 2 ++ > 5 files changed, 27 insertions(+), 11 deletions(-) > > diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c > index 23223575a2..90d8f652d3 100644 > --- a/xen/arch/arm/gic-v2.c > +++ b/xen/arch/arm/gic-v2.c > @@ -51,6 +51,8 @@ > #define GICH_V2_LR_PHYSICAL_SHIFT 10 > #define GICH_V2_LR_STATE_MASK 0x3 > #define GICH_V2_LR_STATE_SHIFT 28 > +#define GICH_V2_LR_PENDING (1U << 28) > +#define GICH_V2_LR_ACTIVE (1U << 29) > #define GICH_V2_LR_PRIORITY_SHIFT 23 > #define GICH_V2_LR_PRIORITY_MASK 0x1f > #define GICH_V2_LR_HW_SHIFT31 > @@ -467,7 +469,8 @@ static void gicv2_read_lr(int lr, struct gic_lr *lr_reg) > lr_reg->pirq = (lrv >> GICH_V2_LR_PHYSICAL_SHIFT) & > GICH_V2_LR_PHYSICAL_MASK; > lr_reg->virq = (lrv >> GICH_V2_LR_VIRTUAL_SHIFT) & > GICH_V2_LR_VIRTUAL_MASK; > lr_reg->priority = (lrv >> GICH_V2_LR_PRIORITY_SHIFT) & > GICH_V2_LR_PRIORITY_MASK; > -lr_reg->state = (lrv >> GICH_V2_LR_STATE_SHIFT) & > GICH_V2_LR_STATE_MASK; > +lr_reg->pending = (lrv & GICH_V2_LR_PENDING) == GICH_V2_LR_PENDING; > +lr_reg->active = (lrv & GICH_V2_LR_ACTIVE) == GICH_V2_LR_ACTIVE; > lr_reg->hw_status = (lrv & GICH_V2_LR_HW) == GICH_V2_LR_HW; > } > > @@ -478,9 +481,13 @@ static void gicv2_write_lr(int lr, const struct gic_lr > *lr_reg) > lrv = ( ((lr_reg->pirq & GICH_V2_LR_PHYSICAL_MASK) << > GICH_V2_LR_PHYSICAL_SHIFT) | >((lr_reg->virq & GICH_V2_LR_VIRTUAL_MASK) << > GICH_V2_LR_VIRTUAL_SHIFT) | >((uint32_t)(lr_reg->priority & GICH_V2_LR_PRIORITY_MASK) > - << GICH_V2_LR_PRIORITY_SHIFT) | > - ((uint32_t)(lr_reg->state & GICH_V2_LR_STATE_MASK) > - << GICH_V2_LR_STATE_SHIFT) ); > + << GICH_V2_LR_PRIORITY_SHIFT) ); > + > +if ( lr_reg->active ) > +lrv |= GICH_V2_LR_ACTIVE; > + > +if ( lr_reg->pending ) > +lrv |= GICH_V2_LR_PENDING; > > if ( lr_reg->hw_status ) > lrv |= GICH_V2_LR_HW; > diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c > index 0711e509a6..4dbbf0afd2 100644 > --- a/xen/arch/arm/gic-v3.c > +++ b/xen/arch/arm/gic-v3.c > @@ -1010,7 +1010,8 @@ static void gicv3_read_lr(int lr, struct gic_lr *lr_reg) > lr_reg->virq = (lrv >> ICH_LR_VIRTUAL_SHIFT) & ICH_LR_VIRTUAL_MASK; > > lr_reg->priority = (lrv >> ICH_LR_PRIORITY_SHIFT) & > ICH_LR_PRIORITY_MASK; > -lr_reg->state = (lrv >> ICH_LR_STATE_SHIFT) & ICH_LR_STATE_MASK; > +lr_reg->pending = (lrv & ICH_LR_STATE_PENDING) == ICH_LR_STATE_PENDING; > +lr_reg->active= (lrv & ICH_LR_STATE_ACTIVE) == ICH_LR_STATE_ACTIVE; > lr_reg->hw_status = (lrv & ICH_LR_HW) == ICH_LR_HW; > } > > @@ -1020,8 +1021,13 @@ static void gicv3_write_lr(int lr_reg, const struct > gic_lr *lr) > > lrv = ( ((u64)(lr->pirq & ICH_LR_PHYSICAL_MASK) << > ICH_LR_PHYSICAL_SHIFT)| > ((u64)(lr->virq & ICH_LR_VIRTUAL_MASK) << ICH_LR_VIRTUAL_SHIFT) | > -((u64)(lr->priority & ICH_LR_PRIORITY_MASK) << > ICH_LR_PRIORITY_SHIFT)| > -((u64)(lr->state & ICH_LR_STATE_MASK) << ICH_LR_STATE_SHIFT) ); > +((u64)(lr->priority & ICH_LR_PRIORITY_MASK) << > ICH_LR_PRIORITY_SHIFT) ); > + > +if ( lr->active ) > +lrv |= ICH_LR_STATE_ACTIVE; > + > +if ( lr->pending ) > +lrv |= ICH_LR_STATE_PENDING; > > if ( lr->hw_status ) > lrv |= ICH_LR_HW; > diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c > index e3cb47e80e..d831b35525 100644 > --- a/xen/arch/arm/gic-vgic.c > +++ b/xen/arch/arm/gic-vgic.c > @@ -189,7 +189,7 @@ static void gic_update_one_lr(struct vcpu *v, int i) > return; > } > > -if ( lr_val.state & GICH_LR_ACTIVE ) > +if ( lr_val.active ) > { > set_bit(GIC_IRQ_GUEST_ACTIVE, &p->status); > if ( test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) && > @@ -197,7 +197,7 @@ static void gic_update_one_lr(struct vcpu *v, int i) > { > if ( p->desc == NULL ) > { > -lr_val.state |= GICH_LR_PENDING; > +lr_val.pending = true; > gic_hw_ops->write_lr(i, &lr_val); > } > else > @@ -205,7 +205,7 @@ static void gic_update_one_lr(struct vcpu *v, int i) > irq, v->domain->domain_id, v->vcpu_id, i); > } > } > -else if ( lr_val.state & GICH_LR_PENDING ) > +else if ( lr_val.pending ) > { > int q __attribute__ ((unused)) =
Re: [Xen-devel] [PATCH 2/6] xen/arm: vgic: Override the group in lr everytime
Hi Andre, On 03/14/2018 06:02 PM, Andre Przywara wrote: On 09/03/18 17:35, julien.gr...@arm.com wrote: From: Julien Grall At the moment, write_lr is assuming the caller will set correctly the group. However the group should always be 0 when the guest is using vGICv2 and 1 for vGICv3. As the caller should not care about the group, override it directly. I think that makes sense, mostly because this is what KVM does as well. And it's a good idea to do this in write_lr(). I understand that this is effectively what I did in the new VGIC, but it would be good to double check that this is the right thing to do for the old VGIC as well. Did you test this on some GICv3 h/w or the model? Or do we always call update_lr() anyway? I had a patch to remove update_lr and use write_lr. I exercised the code with it but didn't send it because of some nasty bug with the priority in write_lr and the old vGIC. On the old vGIC write_lr is only used when in gic_update_one_lr(...) combine with read_lr before. In that context we don't care about the group, so overwriting it is fine and the right thing to do. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/6] xen/arm: gic: Use bool instead of uint8_t for the hw_status in gic_lr
Hi, On 09/03/18 16:35, julien.gr...@arm.com wrote: > From: Julien Grall > > hw_status can only be 1 or 0. So convert to a bool. > > Signed-off-by: Julien Grall Reviewed-by: Andre Przywara Cheers, Andre. > --- > xen/arch/arm/gic-v2.c | 9 + > xen/arch/arm/gic-v3.c | 8 +--- > xen/include/asm-arm/gic.h | 2 +- > 3 files changed, 11 insertions(+), 8 deletions(-) > > diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c > index fc105c08b8..23223575a2 100644 > --- a/xen/arch/arm/gic-v2.c > +++ b/xen/arch/arm/gic-v2.c > @@ -468,7 +468,7 @@ static void gicv2_read_lr(int lr, struct gic_lr *lr_reg) > lr_reg->virq = (lrv >> GICH_V2_LR_VIRTUAL_SHIFT) & > GICH_V2_LR_VIRTUAL_MASK; > lr_reg->priority = (lrv >> GICH_V2_LR_PRIORITY_SHIFT) & > GICH_V2_LR_PRIORITY_MASK; > lr_reg->state = (lrv >> GICH_V2_LR_STATE_SHIFT) & > GICH_V2_LR_STATE_MASK; > -lr_reg->hw_status = (lrv >> GICH_V2_LR_HW_SHIFT) & GICH_V2_LR_HW_MASK; > +lr_reg->hw_status = (lrv & GICH_V2_LR_HW) == GICH_V2_LR_HW; > } > > static void gicv2_write_lr(int lr, const struct gic_lr *lr_reg) > @@ -480,9 +480,10 @@ static void gicv2_write_lr(int lr, const struct gic_lr > *lr_reg) >((uint32_t)(lr_reg->priority & GICH_V2_LR_PRIORITY_MASK) ><< GICH_V2_LR_PRIORITY_SHIFT) | >((uint32_t)(lr_reg->state & GICH_V2_LR_STATE_MASK) > - << GICH_V2_LR_STATE_SHIFT) | > - ((uint32_t)(lr_reg->hw_status & GICH_V2_LR_HW_MASK) > - << GICH_V2_LR_HW_SHIFT)); > + << GICH_V2_LR_STATE_SHIFT) ); > + > +if ( lr_reg->hw_status ) > +lrv |= GICH_V2_LR_HW; > > writel_gich(lrv, GICH_LR + lr * 4); > } > diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c > index 0dfa1a1e08..0711e509a6 100644 > --- a/xen/arch/arm/gic-v3.c > +++ b/xen/arch/arm/gic-v3.c > @@ -1011,7 +1011,7 @@ static void gicv3_read_lr(int lr, struct gic_lr *lr_reg) > > lr_reg->priority = (lrv >> ICH_LR_PRIORITY_SHIFT) & > ICH_LR_PRIORITY_MASK; > lr_reg->state = (lrv >> ICH_LR_STATE_SHIFT) & ICH_LR_STATE_MASK; > -lr_reg->hw_status = (lrv >> ICH_LR_HW_SHIFT) & ICH_LR_HW_MASK; > +lr_reg->hw_status = (lrv & ICH_LR_HW) == ICH_LR_HW; > } > > static void gicv3_write_lr(int lr_reg, const struct gic_lr *lr) > @@ -1021,8 +1021,10 @@ static void gicv3_write_lr(int lr_reg, const struct > gic_lr *lr) > lrv = ( ((u64)(lr->pirq & ICH_LR_PHYSICAL_MASK) << > ICH_LR_PHYSICAL_SHIFT)| > ((u64)(lr->virq & ICH_LR_VIRTUAL_MASK) << ICH_LR_VIRTUAL_SHIFT) | > ((u64)(lr->priority & ICH_LR_PRIORITY_MASK) << > ICH_LR_PRIORITY_SHIFT)| > -((u64)(lr->state & ICH_LR_STATE_MASK) << ICH_LR_STATE_SHIFT) | > -((u64)(lr->hw_status & ICH_LR_HW_MASK) << ICH_LR_HW_SHIFT) ); > +((u64)(lr->state & ICH_LR_STATE_MASK) << ICH_LR_STATE_SHIFT) ); > + > +if ( lr->hw_status ) > +lrv |= ICH_LR_HW; > > /* > * When the guest is using vGICv3, all the IRQs are Group 1. Group 0 > diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h > index 1eb08b856e..daec51499c 100644 > --- a/xen/include/asm-arm/gic.h > +++ b/xen/include/asm-arm/gic.h > @@ -210,7 +210,7 @@ struct gic_lr { > uint32_t virq; > uint8_t priority; > uint8_t state; > - uint8_t hw_status; > + bool hw_status; > }; > > enum gic_version { > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/6] xen/arm: vgic: Override the group in lr everytime
On 09/03/18 17:35, julien.gr...@arm.com wrote: > From: Julien Grall > > At the moment, write_lr is assuming the caller will set correctly the > group. However the group should always be 0 when the guest is using > vGICv2 and 1 for vGICv3. As the caller should not care about the group, > override it directly. I think that makes sense, mostly because this is what KVM does as well. And it's a good idea to do this in write_lr(). I understand that this is effectively what I did in the new VGIC, but it would be good to double check that this is the right thing to do for the old VGIC as well. Did you test this on some GICv3 h/w or the model? Or do we always call update_lr() anyway? Cheers, Andre. > With that change, write_lr is now behaving like update_lr for the group. > > Signed-off-by: Julien Grall > --- > xen/arch/arm/gic-v2.c | 4 +--- > xen/arch/arm/gic-v3.c | 11 --- > xen/include/asm-arm/gic.h | 1 - > 3 files changed, 9 insertions(+), 7 deletions(-) > > diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c > index f16e17c1a3..fc105c08b8 100644 > --- a/xen/arch/arm/gic-v2.c > +++ b/xen/arch/arm/gic-v2.c > @@ -469,7 +469,6 @@ static void gicv2_read_lr(int lr, struct gic_lr *lr_reg) > lr_reg->priority = (lrv >> GICH_V2_LR_PRIORITY_SHIFT) & > GICH_V2_LR_PRIORITY_MASK; > lr_reg->state = (lrv >> GICH_V2_LR_STATE_SHIFT) & > GICH_V2_LR_STATE_MASK; > lr_reg->hw_status = (lrv >> GICH_V2_LR_HW_SHIFT) & GICH_V2_LR_HW_MASK; > -lr_reg->grp = (lrv >> GICH_V2_LR_GRP_SHIFT) & GICH_V2_LR_GRP_MASK; > } > > static void gicv2_write_lr(int lr, const struct gic_lr *lr_reg) > @@ -483,8 +482,7 @@ static void gicv2_write_lr(int lr, const struct gic_lr > *lr_reg) >((uint32_t)(lr_reg->state & GICH_V2_LR_STATE_MASK) > << GICH_V2_LR_STATE_SHIFT) | >((uint32_t)(lr_reg->hw_status & GICH_V2_LR_HW_MASK) > - << GICH_V2_LR_HW_SHIFT) | > - ((uint32_t)(lr_reg->grp & GICH_V2_LR_GRP_MASK) << > GICH_V2_LR_GRP_SHIFT) ); > + << GICH_V2_LR_HW_SHIFT)); > > writel_gich(lrv, GICH_LR + lr * 4); > } > diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c > index 09b49a07d5..0dfa1a1e08 100644 > --- a/xen/arch/arm/gic-v3.c > +++ b/xen/arch/arm/gic-v3.c > @@ -1012,7 +1012,6 @@ static void gicv3_read_lr(int lr, struct gic_lr *lr_reg) > lr_reg->priority = (lrv >> ICH_LR_PRIORITY_SHIFT) & > ICH_LR_PRIORITY_MASK; > lr_reg->state = (lrv >> ICH_LR_STATE_SHIFT) & ICH_LR_STATE_MASK; > lr_reg->hw_status = (lrv >> ICH_LR_HW_SHIFT) & ICH_LR_HW_MASK; > -lr_reg->grp = (lrv >> ICH_LR_GRP_SHIFT) & ICH_LR_GRP_MASK; > } > > static void gicv3_write_lr(int lr_reg, const struct gic_lr *lr) > @@ -1023,8 +1022,14 @@ static void gicv3_write_lr(int lr_reg, const struct > gic_lr *lr) > ((u64)(lr->virq & ICH_LR_VIRTUAL_MASK) << ICH_LR_VIRTUAL_SHIFT) | > ((u64)(lr->priority & ICH_LR_PRIORITY_MASK) << > ICH_LR_PRIORITY_SHIFT)| > ((u64)(lr->state & ICH_LR_STATE_MASK) << ICH_LR_STATE_SHIFT) | > -((u64)(lr->hw_status & ICH_LR_HW_MASK) << ICH_LR_HW_SHIFT) | > -((u64)(lr->grp & ICH_LR_GRP_MASK) << ICH_LR_GRP_SHIFT) ); > +((u64)(lr->hw_status & ICH_LR_HW_MASK) << ICH_LR_HW_SHIFT) ); > + > +/* > + * When the guest is using vGICv3, all the IRQs are Group 1. Group 0 > + * would result in a FIQ, which will not be expected by the guest OS. > + */ > +if ( current->domain->arch.vgic.version == GIC_V3 ) > +lrv |= ICH_LR_GRP1; > > gicv3_ich_write_lr(lr_reg, lrv); > } > diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h > index 49cb94f792..1eb08b856e 100644 > --- a/xen/include/asm-arm/gic.h > +++ b/xen/include/asm-arm/gic.h > @@ -211,7 +211,6 @@ struct gic_lr { > uint8_t priority; > uint8_t state; > uint8_t hw_status; > - uint8_t grp; > }; > > enum gic_version { > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC PATCH 11/12] hvmloader: use libacpi to build MCFG table
On Tue, 13 Mar 2018 04:33:56 +1000 Alexey Gerasimenko wrote: >This patch extends hvmloader_acpi_build_tables() with code which >detects if MMCONFIG is available -- i.e. initialized and enabled >(+we're running on Q35), obtains its base address and size and asks >libacpi to build MCFG table for it via setting the flag ACPI_HAS_MCFG >in a manner similar to other optional ACPI tables building. > >Signed-off-by: Alexey Gerasimenko >--- > tools/firmware/hvmloader/util.c | 70 > + 1 file changed, 70 > insertions(+) Looks like I missed the patch for reserving MMCONFIG area in E820 map, it is required for Linux guests (otherwise MMCONFIG info will be rejected by linux kernel). Windows guests allow to use MMCONFIG without a corresponding E820 entry. Following lines need to be added to /hvmloader/e820.c: +/* mark MMCONFIG area */ +if ( is_mmconfig_used() ) +{ +e820[nr].addr = mmconfig_get_base(); +e820[nr].size = mmconfig_get_size(); +e820[nr].type = E820_RESERVED; +nr++; +} The corresponding patch-file is attached, will include it in v2 patches. hvmloader-mark-MMCONFIG-in-E820-map.patch Description: Binary data ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v10] run QEMU as non-root
On Wed, Mar 14, 2018 at 4:14 PM, Anthony PERARD wrote: > On Wed, Mar 14, 2018 at 02:49:37PM +, George Dunlap wrote: >> On Thu, Nov 5, 2015 at 12:47 PM, Stefano Stabellini >> wrote: >> > diff --git a/docs/misc/qemu-deprivilege.txt >> > b/docs/misc/qemu-deprivilege.txt >> > new file mode 100644 >> > index 000..dde74ab >> > --- /dev/null >> > +++ b/docs/misc/qemu-deprivilege.txt >> > @@ -0,0 +1,31 @@ >> > +For security reasons, libxl tries to pass a non-root username to QEMU as >> > +argument. During initialization QEMU calls setuid and setgid with the >> > +user ID and the group ID of the user passed as argument. >> > +Libxl looks for the following users in this order: >> > + >> > +1) a user named "xen-qemuuser-domid$domid", >> > +Where $domid is the domid of the domain being created. >> > +This requires the reservation of 65535 uids from xen-qemuuser-domid1 >> > +to xen-qemuuser-domid65535. To use this mechanism, you might want to >> > +create a large number of users at installation time. For example: >> > + >> > +for ((i=1; i<65536; i++)) >> > +do >> > +adduser --no-create-home --system xen-qemuuser-domid$i >> > +done >> >> This fails for me after a few hundred uids: >> >> adduser: No UID is available in the range 100-999 (FIRST_SYS_UID - >> LAST_SYS_UID). >> adduser: The user `xen-qemuuser-domid892' was not created. >> adduser: No UID is available in the range 100-999 (FIRST_SYS_UID - >> LAST_SYS_UID). >> adduser: The user `xen-qemuuser-domid893' was not created. >> adduser: No UID is available in the range 100-999 (FIRST_SYS_UID - >> LAST_SYS_UID). >> adduser: The user `xen-qemuuser-domid894' was not created. >> >> It looks like even if --system were omitted, this would fail on a >> normal system, as the default UID range looks to be [1000,2]. >> >> Also, on my test box a single 'adduser' takes about 1 second, meaning >> just doing up to the normal number of domains (around 32k) would take >> 9 hours or so; is that really a practical suggestion? > > Using systemd, it's a bit faster: > > for ((i=1; i<5000; i++)); do > echo "u xen-qemuuser-domid$i -" >> /etc/sysusers.d/xen-qemu-depriv.conf; > done > $ time systemd-sysusers xen-qemu-depriv.conf > systemd-sysusers xen-qemu-depriv.conf 1.19s user 1.71s system 45% cpu 6.351 > total > > but that also fails to create user xen-qemuuser-domid940 and the > following. And uid for all new users was < 1000. > > That works better if you start my little script here with: > echo "r - 2000-9000" >> /etc/sysusers.d/xen-qemu-depriv.conf > # for 5000 uid: > systemd-sysusers xen-qemu-depriv.conf 2.29s user 3.54s system 47% cpu 12.237 > total Well, that may be, but *this particular document* doesn't mention systemd, nor modifying the range of UIDs available; so it's not a practical suggestion. > George, if you read the manual for dm_restrict, the first option > presented is to create a single userid that is the start of a range of > uid to use: Right, I did miss that from the man page -- but again, that option is not mentioned here in this document. A lot of the stuff in the man page there isn't really suitable for a man page; it should be put in a separate document. I'll submit some patches. :-) -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2] xenbaked.c: Avoid divide by zero issue
xenbaked.c -> dump_stats(), run_time = time(&end_time) - time(&start_time), time() returns the value in seconds. If one cancels xenmon.py immediately after started, run_time can be zero, and then xenbaked will hit divide by zero fault. Signed-off-by: Joe Jin Reviewed-by: Konrad Rzeszutek Wilk --- tools/xenmon/xenbaked.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tools/xenmon/xenbaked.c b/tools/xenmon/xenbaked.c index 3d9e0ed900..d3f940a26b 100644 --- a/tools/xenmon/xenbaked.c +++ b/tools/xenmon/xenbaked.c @@ -243,10 +243,12 @@ static void dump_stats(void) } printf("processed %d total records in %d seconds (%ld per second)\n", - rec_count, (int)run_time, (long)(rec_count/run_time)); + rec_count, (int)run_time, + run_time ? (long)(rec_count/run_time) : 0L); -printf("woke up %d times in %d seconds (%ld per second)\n", wakeups, - (int) run_time, (long)(wakeups/run_time)); +printf("woke up %d times in %d seconds (%ld per second)\n", + wakeups, (int) run_time, + run_time ? (long)(wakeups/run_time) : 0L); check_gotten_sum(); } -- 2.14.3 (Apple Git-98) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 1/4] x86/PVHv2: Add memory map pointer to hvm_start_info struct
On 03/14/2018 03:55 AM, Jan Beulich wrote: On 14.03.18 at 00:31, wrote: >> + * For x86 implementations at least, the values used in the type field will >> + * match the Address Range Types as defined in section 15 (System Address >> + * Map Interfaces) of the ACPI Specification >> (http://uefi.org/specifications) >> + * where: >> + * AddressRangeMemory = 1 (E820_RAM) >> + * AddressRangeReserved = 2 (E820_RESERVED) >> + * AddressRangeACPI = 3 (E820_ACPI) >> + * AddressRangeNVS = 4 (E820_NVS) >> + * AddressRangeUnusable = 5 (E820_UNUSABLE) >> + * AddressRangeDisabled = 6 (E820_DISABLED) >> + * AddressRangePersistentMemory = 7 (E820_PMEM) > Would you mind waiting for a discussion to settle before sending > out new patch versions? As indicated in an earlier reply to v1, I > consider this still insufficient. And no, I'm not asking for you to > add redundant and potentially conflicting definitions of E820_*, > but instead you want to use Xen specific ones (prefixed e.g. > by XEN_HVM_MEMMAP_TYPE_). Since we will now have a non-Xen user of this interface perhaps PVH_MEMMAP_TYPE_ ? -boris ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [SVM] Getting the length of the current instruction in svm_vmexit_handler()
On 14/03/18 15:53, Jan Beulich wrote: On 14.03.18 at 15:56, wrote: >> We'd like to retrieve the length of the current instruction in >> svm_vmexit_handler(), specifically for the VMEXIT_EXCEPTION_DB and >> VMEXIT_EXCEPTION_BP cases. >> >> We've combed the vmcb to no avail. Everything we've thought to check >> (exitinfo1, exitinfo2, exitintinfo) turns out to be zero there while >> testing. >> >> There's __get_instruction_length(vcpu, instr), but it expects to be fed >> the exact instruction we want the length for, which obviously defeats >> the purpose here. >> >> Is there a clean way to get the current instruction length like we do in >> the VMX case (__vmread(VM_EXIT_INSTRUCTION_LEN, &insn_len)) that we're >> overlooking? > Just like Intel's, AMD's is available in a subset of cases only > (look for vmcb->guest_ins_len), which don't include the > exception intercepts you talk about. For #DB I think there's > no difference between both anyway. On non-first-gen hardware, the difference between RIP and NextRIP should give you the instruction length. ISTR NextRIP is written on all exits, and consumed on all entries. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke bisection] complete build-armhf
branch xen-unstable-smoke xenbranch xen-unstable-smoke job build-armhf testid xen-build 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: b43501451733193b265de30fd79a764363a2a473 Bug not present: eef83fd2af0d4c78afec34c199c977fc97d8a0b3 Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/120741/ commit b43501451733193b265de30fd79a764363a2a473 Author: Doug Goldstein Date: Mon Mar 12 23:06:51 2018 -0500 tools: detect appropriate debug optimization level When building debug use -Og as the optimization level if its available, otherwise retain the use of -O0. -Og has been added by GCC to enable all optimizations that to not affect debugging while retaining full debugability. Signed-off-by: Doug Goldstein Acked-by: Wei Liu For bisection revision-tuple graph see: http://logs.test-lab.xenproject.org/osstest/results/bisect/xen-unstable-smoke/build-armhf.xen-build.html Revision IDs in each graph node refer, respectively, to the Trees above. Running cs-bisection-step --graph-out=/home/logs/results/bisect/xen-unstable-smoke/build-armhf.xen-build --summary-out=tmp/120754.bisection-summary --basis-template=120679 --blessings=real,real-bisect xen-unstable-smoke build-armhf xen-build Searching for failure / basis pass: 120745 fail [host=arndale-westfield] / 120679 ok. Failure / basis pass flights: 120745 / 120679 Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: xen git://xenbits.xen.org/xen.git Latest 5c3fdee026a204a59cb392e43a313ab558de9682 db0c7dde021c29c2ae0d847d70fb7b59e02ea522 Basis pass 5c3fdee026a204a59cb392e43a313ab558de9682 eef83fd2af0d4c78afec34c199c977fc97d8a0b3 Generating revisions with ./adhoc-revtuple-generator git://xenbits.xen.org/qemu-xen.git#5c3fdee026a204a59cb392e43a313ab558de9682-5c3fdee026a204a59cb392e43a313ab558de9682 git://xenbits.xen.org/xen.git#eef83fd2af0d4c78afec34c199c977fc97d8a0b3-db0c7dde021c29c2ae0d847d70fb7b59e02ea522 Loaded 1001 nodes in revision graph Searching for test results: 120679 pass 5c3fdee026a204a59cb392e43a313ab558de9682 eef83fd2af0d4c78afec34c199c977fc97d8a0b3 120685 fail 5c3fdee026a204a59cb392e43a313ab558de9682 a7313da7f7767984172873adf645eff9bd667bda 120688 fail 5c3fdee026a204a59cb392e43a313ab558de9682 a7313da7f7767984172873adf645eff9bd667bda 120699 [host=arndale-lakeside] 120709 [host=arndale-metrocentre] 120741 fail 5c3fdee026a204a59cb392e43a313ab558de9682 b43501451733193b265de30fd79a764363a2a473 120711 [host=arndale-lakeside] 120742 [host=arndale-metrocentre] 120714 [host=arndale-lakeside] 120744 [host=arndale-metrocentre] 120720 [host=arndale-metrocentre] 120723 [host=arndale-metrocentre] 120721 fail 5c3fdee026a204a59cb392e43a313ab558de9682 a7313da7f7767984172873adf645eff9bd667bda 120724 [host=arndale-metrocentre] 120728 pass 5c3fdee026a204a59cb392e43a313ab558de9682 eef83fd2af0d4c78afec34c199c977fc97d8a0b3 120729 fail 5c3fdee026a204a59cb392e43a313ab558de9682 a7313da7f7767984172873adf645eff9bd667bda 120745 fail 5c3fdee026a204a59cb392e43a313ab558de9682 db0c7dde021c29c2ae0d847d70fb7b59e02ea522 120747 [host=arndale-metrocentre] 120730 fail 5c3fdee026a204a59cb392e43a313ab558de9682 b43501451733193b265de30fd79a764363a2a473 120737 pass 5c3fdee026a204a59cb392e43a313ab558de9682 eef83fd2af0d4c78afec34c199c977fc97d8a0b3 120738 fail 5c3fdee026a204a59cb392e43a313ab558de9682 b43501451733193b265de30fd79a764363a2a473 120740 pass 5c3fdee026a204a59cb392e43a313ab558de9682 eef83fd2af0d4c78afec34c199c977fc97d8a0b3 120733 [host=arndale-metrocentre] 120750 pass 5c3fdee026a204a59cb392e43a313ab558de9682 eef83fd2af0d4c78afec34c199c977fc97d8a0b3 120754 fail 5c3fdee026a204a59cb392e43a313ab558de9682 db0c7dde021c29c2ae0d847d70fb7b59e02ea522 Searching for interesting versions Result found: flight 120679 (pass), for basis pass Result found: flight 120745 (fail), for basis failure Repro found: flight 120750 (pass), for basis pass Repro found: flight 120754 (fail), for basis failure 0 revisions at 5c3fdee026a204a59cb392e43a313ab558de9682 eef83fd2af0d4c78afec34c199c977fc97d8a0b3 No revisions left to test, checking graph state. Result found: flight 120679 (pass), for last pass Result found: flight 120730 (fail), for first failure Repro found: flight 120737 (pass), for last pass Repro found: flight 120738 (fail), for first failure Repro found: flight 120740 (pass), for last pass Repro found: flight 120741 (fail), for first failure *** Found and reproduced problem changeset *** Bug is in tree: xen git://xenbits.xen.org/xen.git Bug introduced: b43501451733193b265de30fd79a764363a2a473 Bug not present: eef83fd2af0d4c78afec34c199c977fc97d8a0b3 Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/120741/ commit b43501451733193b265d
[Xen-devel] Implementaiton of cc-option in Config.mk
Hi all The implementation of cc-option (grepping the option being tested in output) in Config.mk now makes it not possible to detect if -Og is supported because "-Og" doesn't appear in the output if it is not supported. I suspect there will be other options that cc-option can't work with. It is implemented like that because we try to detect -Wno-* option. But why is that important? Can't we just ignore -Wno-* if they aren't supported? Does anyone has an idea how to make cc-option work for -Og (and other options)? Thanks, Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 06/27] x86/entry/64: Adapt assembly for PIE support
On Wed, Mar 14, 2018 at 8:55 AM Christopher Lameter wrote: > On Wed, 14 Mar 2018, Peter Zijlstra wrote: > > On Tue, Mar 13, 2018 at 01:59:24PM -0700, Thomas Garnier wrote: > > > @@ -1576,7 +1578,9 @@ first_nmi: > > > addq$8, (%rsp) /* Fix up RSP */ > > > pushfq /* RFLAGS */ > > > pushq $__KERNEL_CS/* CS */ > > > - pushq $1f /* RIP */ > > > + pushq %rax/* Support Position Independent Code */ > > > + leaq1f(%rip), %rax /* RIP */ > > > + xchgq %rax, (%rsp)/* Restore RAX, put 1f */ > > > iretq /* continues at repeat_nmi below */ > > > UNWIND_HINT_IRET_REGS > > > 1: > > > > Urgh, xchg with a memop has an implicit LOCK prefix. > this_cpu_xchg uses no lock cmpxchg as a replacement to reduce latency. Great, I will update my implementation. Thanks Peter and Christoph. > From linux/arch/x86/include/asm/percpu.h > /* > * xchg is implemented using cmpxchg without a lock prefix. xchg is > * expensive due to the implied lock prefix. The processor cannot prefetch > * cachelines if xchg is used. > */ > #define percpu_xchg_op(var, nval) \ > ({ \ > typeof(var) pxo_ret__; \ > typeof(var) pxo_new__ = (nval); \ > switch (sizeof(var)) { \ > case 1: \ > asm("\n\tmov "__percpu_arg(1)",%%al"\ > "\n1:\tcmpxchgb %2, "__percpu_arg(1)\ > "\n\tjnz 1b"\ > : "=&a" (pxo_ret__), "+m" (var) \ > : "q" (pxo_new__) \ > : "memory");\ > break; \ > case 2: \ > asm("\n\tmov "__percpu_arg(1)",%%ax"\ > "\n1:\tcmpxchgw %2, "__percpu_arg(1)\ > "\n\tjnz 1b"\ > : "=&a" (pxo_ret__), "+m" (var) \ > : "r" (pxo_new__) \ > : "memory");\ > break; \ > case 4: \ > asm("\n\tmov "__percpu_arg(1)",%%eax" \ > "\n1:\tcmpxchgl %2, "__percpu_arg(1)\ > "\n\tjnz 1b"\ > : "=&a" (pxo_ret__), "+m" (var) \ > : "r" (pxo_new__) \ > : "memory");\ > break; \ > case 8: \ > asm("\n\tmov "__percpu_arg(1)",%%rax" \ > "\n1:\tcmpxchgq %2, "__percpu_arg(1)\ > "\n\tjnz 1b"\ > : "=&a" (pxo_ret__), "+m" (var) \ > : "r" (pxo_new__) \ > : "memory");\ > break; \ > default: __bad_percpu_size(); \ > } \ > pxo_ret__; \ -- Thomas ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] docs: Fix entry for the "usbdev" option
On 14/03/18 16:00, Anthony PERARD wrote: > The man for xl.cfg have the "devtype=hostdev" option, but xl only > understand "type=hostdev", fix the manual to reflect actual > implementation. > > Signed-off-by: Anthony PERARD Reviewed-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] docs: Fix entry for the "usbdev" option
On 03/14/2018 03:00 PM, Anthony PERARD wrote: > The man for xl.cfg have the "devtype=hostdev" option, but xl only > understand "type=hostdev", fix the manual to reflect actual > implementation. > > Signed-off-by: Anthony PERARD 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 v9 11/11] vpci/msix: add MSI-X handlers
>>> On 14.03.18 at 15:04, wrote: > --- a/xen/arch/x86/hvm/dom0_build.c > +++ b/xen/arch/x86/hvm/dom0_build.c > @@ -1117,7 +1117,7 @@ int __init dom0_construct_pvh(struct domain *d, const > module_t *image, > > pvh_setup_mmcfg(d); > > -panic("Building a PVHv2 Dom0 is not yet supported."); > +printk("WARNING: PVH is an experimental mode with limited > functionality\n"); > return 0; > } Does this need to be accompanied by a new entry in SUPPORT.md, as PVH Dom0 becomes usable now? Otoh issues with Dom0 support aren't normally security issues. > +void vpci_msix_arch_print(const struct vpci_msix *msix) > +{ > +unsigned int i; > + > +for ( i = 0; i < msix->max_entries; i++ ) > +{ > +const struct vpci_msix_entry *entry = &msix->entries[i]; > + > +printk("%6u vec=%02x%7s%6s%3sassert%5s%7s dest_id=%lu mask=%u pirq: > %d\n", > + i, MASK_EXTR(entry->data, MSI_DATA_VECTOR_MASK), > + entry->data & MSI_DATA_DELIVERY_LOWPRI ? "lowest" : "fixed", > + entry->data & MSI_DATA_TRIGGER_LEVEL ? "level" : "edge", > + entry->data & MSI_DATA_LEVEL_ASSERT ? "" : "de", > + entry->addr & MSI_ADDR_DESTMODE_LOGIC ? "log" : "phys", > + entry->addr & MSI_ADDR_REDIRECTION_LOWPRI ? "lowest" : > "fixed", > + MASK_EXTR(entry->addr, MSI_ADDR_DEST_ID_MASK), > + entry->masked, entry->arch.pirq); > +if ( !(i % 50) ) Please use a number such that the compiler can convert this to a shift. > +process_pending_softirqs(); Careful - is this valid with a spin lock held? Note how e.g. dump_domains() holds an RCU lock only. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 16/16] xen: Convert page_to_mfn and mfn_to_page to use typesafe MFN
Hi Jan, On 03/02/2018 04:08 PM, Jan Beulich wrote: On 21.02.18 at 15:02, wrote: --- a/xen/arch/x86/pv/emul-priv-op.c +++ b/xen/arch/x86/pv/emul-priv-op.c @@ -43,16 +43,6 @@ #include "emulate.h" #include "mm.h" -/* Override macros from asm/page.h to make them work with mfn_t */ -#undef mfn_to_page -#define mfn_to_page(mfn) __mfn_to_page(mfn_x(mfn)) -#undef page_to_mfn -#define page_to_mfn(pg) _mfn(__page_to_mfn(pg)) - -/*** - * I/O emulation support - */ Why does this comment go away? From an earlier review, Andrew said: "If you're making this change, please take out the Descriptor Tables comment like you do with I/O below, because the entire file is dedicated to descriptor table support and it will save me one item on a cleanup patch :)." The descriptor one got remove by 634afe43ac "x86/pv: Rename invalidate_shadow_ldt() to pv_destroy_ldt()". So it is not part of this patch anymore. @@ -478,10 +478,10 @@ extern paddr_t mem_hotplug; #define SHARED_M2P(_e) ((_e) == SHARED_M2P_ENTRY) #define compat_machine_to_phys_mapping ((unsigned int *)RDWR_COMPAT_MPT_VIRT_START) -#define _set_gpfn_from_mfn(mfn, pfn) ({\ -struct domain *d = page_get_owner(__mfn_to_page(mfn)); \ -unsigned long entry = (d && (d == dom_cow)) ? \ -SHARED_M2P_ENTRY : (pfn); \ +#define _set_gpfn_from_mfn(mfn, pfn) ({ \ +struct domain *d = page_get_owner(mfn_to_page(_mfn(mfn)));\ +unsigned long entry = (d && (d == dom_cow)) ? \ +SHARED_M2P_ENTRY : (pfn); \ Please don't break the alignment of the backslashes here. It also looks like three of the four lines could be left alone altogether. I am not sure why I modified the 3 other lines. I fixed it. @@ -157,10 +157,10 @@ static inline l4_pgentry_t l4e_from_paddr(paddr_t pa, unsigned int flags) #define l4e_from_intpte(intpte)((l4_pgentry_t) { (intpte_t)(intpte) }) /* Construct a pte from a page pointer and access flags. */ -#define l1e_from_page(page, flags) l1e_from_pfn(__page_to_mfn(page), (flags)) -#define l2e_from_page(page, flags) l2e_from_pfn(__page_to_mfn(page), (flags)) -#define l3e_from_page(page, flags) l3e_from_pfn(__page_to_mfn(page), (flags)) -#define l4e_from_page(page, flags) l4e_from_pfn(__page_to_mfn(page), (flags)) +#define l1e_from_page(page, flags) l1e_from_mfn(page_to_mfn(page), (flags)) +#define l2e_from_page(page, flags) l2e_from_mfn(page_to_mfn(page), (flags)) +#define l3e_from_page(page, flags) l3e_from_mfn(page_to_mfn(page), (flags)) +#define l4e_from_page(page, flags) l4e_from_mfn(page_to_mfn(page), (flags)) Would again have been nice if you got rid of the extra parentheses here at the same time. I admit, I don't spend my time trying to find the possible cleanup in the x86 code. I just do mechanical change and when I get bored I do a bit more. @@ -240,12 +240,12 @@ void copy_page_sse2(void *, const void *); #define __mfn_to_virt(mfn) (maddr_to_virt((paddr_t)(mfn) << PAGE_SHIFT)) /* Convert between machine frame numbers and page-info structures. */ -#define __mfn_to_page(mfn) (frame_table + pfn_to_pdx(mfn)) -#define __page_to_mfn(pg) pdx_to_pfn((unsigned long)((pg) - frame_table)) +#define mfn_to_page(mfn)(frame_table + mfn_to_pdx(mfn)) +#define page_to_mfn(pg) pdx_to_mfn((unsigned long)((pg) - frame_table)) /* Convert between machine addresses and page-info structures. */ -#define __maddr_to_page(ma) __mfn_to_page((ma) >> PAGE_SHIFT) -#define __page_to_maddr(pg) ((paddr_t)__page_to_mfn(pg) << PAGE_SHIFT) +#define __maddr_to_page(ma) mfn_to_page(maddr_to_mfn(ma)) +#define __page_to_maddr(pg) (mfn_to_maddr(page_to_mfn(pg))) Same here. With at least the first two items taken care of, relevant x86 pieces Acked-by: Jan Beulich I don't plan to address the first one as Andrew were happy with it. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v9 09/11] vpci/msi: add MSI handlers
>>> On 14.03.18 at 15:04, wrote: > --- a/xen/drivers/vpci/vpci.c > +++ b/xen/drivers/vpci/vpci.c > @@ -47,6 +47,10 @@ void vpci_remove_device(struct pci_dev *pdev) > xfree(r); > } > spin_unlock(&pdev->vpci->lock); > +#ifdef __XEN__ > +/* NB: fields below are not exposed to the user-space test harness. */ > +xfree(pdev->vpci->msi); > +#endif Would it maybe be better to add such dummy field(s), to avoid the #ifdef here? Anyway, with or without that 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 v10] run QEMU as non-root
On Wed, Mar 14, 2018 at 02:49:37PM +, George Dunlap wrote: > On Thu, Nov 5, 2015 at 12:47 PM, Stefano Stabellini > wrote: > > diff --git a/docs/misc/qemu-deprivilege.txt b/docs/misc/qemu-deprivilege.txt > > new file mode 100644 > > index 000..dde74ab > > --- /dev/null > > +++ b/docs/misc/qemu-deprivilege.txt > > @@ -0,0 +1,31 @@ > > +For security reasons, libxl tries to pass a non-root username to QEMU as > > +argument. During initialization QEMU calls setuid and setgid with the > > +user ID and the group ID of the user passed as argument. > > +Libxl looks for the following users in this order: > > + > > +1) a user named "xen-qemuuser-domid$domid", > > +Where $domid is the domid of the domain being created. > > +This requires the reservation of 65535 uids from xen-qemuuser-domid1 > > +to xen-qemuuser-domid65535. To use this mechanism, you might want to > > +create a large number of users at installation time. For example: > > + > > +for ((i=1; i<65536; i++)) > > +do > > +adduser --no-create-home --system xen-qemuuser-domid$i > > +done > > This fails for me after a few hundred uids: > > adduser: No UID is available in the range 100-999 (FIRST_SYS_UID - > LAST_SYS_UID). > adduser: The user `xen-qemuuser-domid892' was not created. > adduser: No UID is available in the range 100-999 (FIRST_SYS_UID - > LAST_SYS_UID). > adduser: The user `xen-qemuuser-domid893' was not created. > adduser: No UID is available in the range 100-999 (FIRST_SYS_UID - > LAST_SYS_UID). > adduser: The user `xen-qemuuser-domid894' was not created. > > It looks like even if --system were omitted, this would fail on a > normal system, as the default UID range looks to be [1000,2]. > > Also, on my test box a single 'adduser' takes about 1 second, meaning > just doing up to the normal number of domains (around 32k) would take > 9 hours or so; is that really a practical suggestion? Using systemd, it's a bit faster: for ((i=1; i<5000; i++)); do echo "u xen-qemuuser-domid$i -" >> /etc/sysusers.d/xen-qemu-depriv.conf; done $ time systemd-sysusers xen-qemu-depriv.conf systemd-sysusers xen-qemu-depriv.conf 1.19s user 1.71s system 45% cpu 6.351 total but that also fails to create user xen-qemuuser-domid940 and the following. And uid for all new users was < 1000. That works better if you start my little script here with: echo "r - 2000-9000" >> /etc/sysusers.d/xen-qemu-depriv.conf # for 5000 uid: systemd-sysusers xen-qemu-depriv.conf 2.29s user 3.54s system 47% cpu 12.237 total George, if you read the manual for dm_restrict, the first option presented is to create a single userid that is the start of a range of uid to use: > Ideally, set aside a range of 32752 uids (from N to N+32751) and > create a user whose name is xen-qemuuser-range-base and whose uid is N > and whose gid is a plain unprivileged gid. libxl will use one such > user for each domid. Which works fine. -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v9 07/11] vpci/bars: add handlers to map the BARs
>>> On 14.03.18 at 15:04, wrote: > +static void modify_decoding(const struct pci_dev *pdev, bool map, bool > rom_only) > +{ > +struct vpci_header *header = &pdev->vpci->header; > +uint8_t slot = PCI_SLOT(pdev->devfn), func = PCI_FUNC(pdev->devfn); > +unsigned int i; > + > +for ( i = 0; i < ARRAY_SIZE(header->bars); i++ ) > +{ > +if ( rom_only && header->bars[i].type == VPCI_BAR_ROM ) > +{ > +unsigned int rom_pos = (i == PCI_HEADER_NORMAL_NR_BARS) > + ? PCI_ROM_ADDRESS : PCI_ROM_ADDRESS1; > +uint32_t val = pci_conf_read32(pdev->seg, pdev->bus, slot, func, > + rom_pos); > + > +header->bars[i].enabled = header->rom_enabled = map; > + > +val &= ~PCI_ROM_ADDRESS_ENABLE; > +val |= map ? PCI_ROM_ADDRESS_ENABLE : 0; > +pci_conf_write32(pdev->seg, pdev->bus, slot, func, rom_pos, val); > +break; > +} > +if ( !rom_only && (header->bars[i].type != VPCI_BAR_ROM || > + header->rom_enabled) ) > +header->bars[i].enabled = map; While this second if() has benefited from the rename to "rom_only", I'm now wondering about the validity of the first if(): Why would this need doing only in the "ROM only" case, but not in the "everything" one? Or is the parameter still suffering from its name being misleading? This also needs to be viewed in context of the call here from vpci_process_pending(), which passes (dropping the conditional there) v->vpci.rom, which doesn't exactly mean "ROM only". If there's really no name for the parameter that can properly convey its meaning, please attach a clarifying comment. (Having reached the end of the patch I now seem to understand / recall that this is for the case where the ROM BAR's enable bit changes. That's what a comment could usefully say here.) > +} > + > +if ( !rom_only ) > +{ Note how due to this conditional the "break" above could actually be "return", making more obvious that the rest of the function isn't be needed in that case. > +static int maybe_defer_map(struct domain *d, struct pci_dev *pdev, > + struct rangeset *mem, bool map, bool rom) > +{ > +struct vcpu *curr = current; > +int rc; > + > +if ( is_idle_vcpu(curr) ) > +{ > +struct map_data data = { .d = d, .map = true }; > + > +/* > + * Dom0 building runs on the idle vCPU, in which case it's not > possible > + * to defer the operation (like done in the else branch). Call > + * rangeset_consume_ranges in order to establish the mappings right > + * away. > + */ For one I think this comment belongs above the if(), as that's what it explains, not the ASSERT() that follows. And then it clarifies only half of what needs clarifying: Why can't we make it here on an idle vCPU outside of Dom0 building (e.g. through a tasklet), or if we can, why is the given behavior the intended one? > +ASSERT(map && !rom); I can see why you assume it's not an un-mapping request (albeit I wonder whether you couldn't instead simply set .map above to the incoming value), but why the !rom part? > +static int modify_bars(const struct pci_dev *pdev, bool map, bool rom_only) > +{ > +struct vpci_header *header = &pdev->vpci->header; > +struct rangeset *mem = rangeset_new(NULL, NULL, 0); > +struct pci_dev *tmp, *dev = NULL; > +unsigned int i; > +int rc; > + > +if ( !mem ) > +return -ENOMEM; > + > +/* > + * Create a rangeset that represents the current device BARs memory > region > + * and compare it against all the currently active BAR memory regions. If > + * an overlap is found, subtract it from the region to be > + * mapped/unmapped. > + * > + * NB: the rangeset uses inclusive frame numbers. Is this a worthwhile remark to make? All rangesets do, so if at all that's what the comment should say. > + */ > + > +/* > + * First fill the rangeset with all the BARs of this device or with the > ROM > + * BAR only, depending on whether the guest is toggling the memory decode > + * bit of the command register, or the enable bit of the ROM BAR > register. > + */ > +for ( i = 0; i < ARRAY_SIZE(header->bars); i++ ) > +{ > +const struct vpci_bar *bar = &header->bars[i]; > + > +if ( !MAPPABLE_BAR(bar) || > + (rom_only ? bar->type != VPCI_BAR_ROM > + : (bar->type == VPCI_BAR_ROM && > !header->rom_enabled)) ) > +continue; > + > +rc = rangeset_add_range(mem, PFN_DOWN(bar->addr), > +PFN_UP(bar->addr + bar->size - 1)); > +if ( rc ) > +{ > +printk(XENLOG_G_WARNING > + "Failed to add [%" PRI_gfn ", %" PRI_gfn "]: %d\n", > + PFN_DOWN(bar->addr), PFN_
Re: [Xen-devel] [PATCH v4 12/16] xen/mm: Switch common/memory.c to use typesafe MFN
Hi Jan, On 03/12/2018 06:39 AM, Jan Beulich wrote: Julien Grall 03/11/18 8:44 PM >>> On 03/09/2018 05:33 PM, Wei Liu wrote: On Mon, Mar 05, 2018 at 07:41:54AM -0700, Jan Beulich wrote: On 05.03.18 at 15:18, wrote: Also, do you have an opinion on Wei's suggestion: "What I meant was to make copy_{to,from}_guest* type-safe. I just feel it a bit strange you only created a wrapper for this file. I wonder why. Note I'm just asking question. That's not necessarily a good idea to turn them all in the end." Well, I didn't really understand what he's after (in the context of this series) - copy_{to,from}_guest() don't take or return MFNs or GFNs. Fundamentally Julien's patch is to wrap around an existing API for this one file only. Why is this file special? Why not just make that class of APIs do what he wants? But that is going to be intrusive and a bit counter-intuitive. I have quickly looked at it. The major problem I can see is it is not possible to generically define for any typesafe. Indeed, TYPE_SAFE(...) cannot define new macro and, AFAICT, it is not feasible to define static inline for copy_* helpers. So we would need to introduce macros for each typesafe by hand. I can move copy_mfn_to_guest in xen/mm.h if people think it could be useful. First of all - how often do we copy in/out individual MFNs? Not in many places, I think. Hence I'm afraid I continue to not see the value of such a construct, especially not as a wider-than-file-scope one. I think you are right. Looking at the interface, we tend copy copy more often a GFN. We might want to provide an helper for typesafe GFN in the future. Cheers. -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 2/3] sndif: Add explicit back and front synchronization
From: Oleksandr Andrushchenko In order to provide explicit synchronization between backend and frontend the following changes are introduced in the protocol: - bump protocol version to 2 - add new ring buffer for sending asynchronous events from backend to frontend to report number of bytes played by the frontend (XENSND_EVT_CUR_POS) - introduce trigger events for playback control: start/stop/pause/resume - add "req-" prefix to event-channel and ring-ref to unify naming of the Xen event channels for requests and events Signed-off-by: Oleksandr Andrushchenko Signed-off-by: Oleksandr Grytsov Cc: Konrad Rzeszutek Wilk Cc: Takashi Iwai Cc: Takashi Sakamoto Cc: Clemens Ladisch --- xen/include/public/io/sndif.h | 168 +- 1 file changed, 164 insertions(+), 4 deletions(-) diff --git a/xen/include/public/io/sndif.h b/xen/include/public/io/sndif.h index 667e610fda2b..1a6a1467f253 100644 --- a/xen/include/public/io/sndif.h +++ b/xen/include/public/io/sndif.h @@ -41,7 +41,7 @@ * Protocol version ** */ -#define XENSND_PROTOCOL_VERSION 1 +#define XENSND_PROTOCOL_VERSION 2 /* ** @@ -113,6 +113,8 @@ * * /local/domain/1/device/vsnd/0/0/0/ring-ref = "386" * /local/domain/1/device/vsnd/0/0/0/event-channel = "15" + * /local/domain/1/device/vsnd/0/0/0/evt-ring-ref = "1386" + * /local/domain/1/device/vsnd/0/0/0/evt-event-channel = "215" * *-- Stream 1, capture * @@ -122,6 +124,8 @@ * * /local/domain/1/device/vsnd/0/0/1/ring-ref = "384" * /local/domain/1/device/vsnd/0/0/1/event-channel = "13" + * /local/domain/1/device/vsnd/0/0/1/evt-ring-ref = "1384" + * /local/domain/1/device/vsnd/0/0/1/evt-event-channel = "213" * *--- PCM device 1 * @@ -135,6 +139,8 @@ * * /local/domain/1/device/vsnd/0/1/0/ring-ref = "387" * /local/domain/1/device/vsnd/0/1/0/event-channel = "151" + * /local/domain/1/device/vsnd/0/1/0/evt-ring-ref = "1387" + * /local/domain/1/device/vsnd/0/1/0/evt-event-channel = "351" * *--- PCM device 2 * @@ -147,6 +153,8 @@ * * /local/domain/1/device/vsnd/0/2/0/ring-ref = "389" * /local/domain/1/device/vsnd/0/2/0/event-channel = "152" + * /local/domain/1/device/vsnd/0/2/0/evt-ring-ref = "1389" + * /local/domain/1/device/vsnd/0/2/0/evt-event-channel = "452" * ** *Backend XenBus Nodes @@ -292,6 +300,23 @@ * The Xen grant reference granting permission for the backend to map * a sole page in a single page sized ring buffer. * + *- Stream Event Transport Parameters - + * + * This communication path is used to deliver asynchronous events from backend + * to frontend, set up per stream. + * + * evt-event-channel + * Values: + * + * The identifier of the Xen event channel used to signal activity + * in the ring buffer. + * + * evt-ring-ref + * Values: + * + * The Xen grant reference granting permission for the backend to map + * a sole page in a single page sized ring buffer. + * ** * STATE DIAGRAMS ** @@ -439,6 +464,19 @@ #define XENSND_OP_GET_VOLUME5 #define XENSND_OP_MUTE 6 #define XENSND_OP_UNMUTE7 +#define XENSND_OP_TRIGGER 8 + +#define XENSND_OP_TRIGGER_START 0 +#define XENSND_OP_TRIGGER_PAUSE 1 +#define XENSND_OP_TRIGGER_STOP 2 +#define XENSND_OP_TRIGGER_RESUME3 + +/* + ** + * EVENT CODES + ** + */ +#define XENSND_EVT_CUR_POS 0 /* ** @@ -455,6 +493,8 @@ #define XENSND_FIELD_VCARD_LONG_NAME"long-name" #define XENSND_FIELD_RING_REF "ring-ref" #define XENSND_FIELD_EVT_CHNL "event-channel" +#define XENSND_FIELD_EVT_RING_REF "evt-ring-ref" +#define XENSND_FIELD_EVT_EVT_CHNL "evt-event-channel" #define XENSND_FIELD_DEVICE_NAME"name" #define XENSND_FIELD_TYPE "type" #define XENSND_FIELD_STREAM_UNIQUE_ID "unique-id" @@ -566,9 +606,7 @@ * +++++ * |
[Xen-devel] [PATCH v2 1/3] sndif: Introduce protocol version
From: Oleksandr Andrushchenko Protocol version was referenced in the protocol description, but missed its definition. Fix this by adding a constant for current protocol version. Signed-off-by: Oleksandr Andrushchenko --- xen/include/public/io/sndif.h | 7 +++ 1 file changed, 7 insertions(+) diff --git a/xen/include/public/io/sndif.h b/xen/include/public/io/sndif.h index c5c1978406b3..667e610fda2b 100644 --- a/xen/include/public/io/sndif.h +++ b/xen/include/public/io/sndif.h @@ -38,6 +38,13 @@ /* ** + * Protocol version + ** + */ +#define XENSND_PROTOCOL_VERSION 1 + +/* + ** * Feature and Parameter Negotiation ** * -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 3/3] sndif: Add explicit back and front parameter negotiation
From: Oleksandr Andrushchenko In order to provide explicit stream parameter negotiation between backend and frontend the following changes are introduced in the protocol: add XENSND_OP_HW_PARAM_QUERY request to read/update configuration space for the parameter given: request passes desired parameter interval (mask) and the response to this request returns min/max interval (mask) for the parameter to be used. Parameters supported by this request/response: - format mask - sample rate interval - number of channels interval - buffer size, interval, frames - period size, interval, frames Signed-off-by: Oleksandr Andrushchenko Cc: Konrad Rzeszutek Wilk Cc: Takashi Iwai --- xen/include/public/io/sndif.h | 124 +++--- 1 file changed, 117 insertions(+), 7 deletions(-) diff --git a/xen/include/public/io/sndif.h b/xen/include/public/io/sndif.h index 1a6a1467f253..ff0ba15b0462 100644 --- a/xen/include/public/io/sndif.h +++ b/xen/include/public/io/sndif.h @@ -465,12 +465,19 @@ #define XENSND_OP_MUTE 6 #define XENSND_OP_UNMUTE7 #define XENSND_OP_TRIGGER 8 +#define XENSND_OP_HW_PARAM_QUERY9 #define XENSND_OP_TRIGGER_START 0 #define XENSND_OP_TRIGGER_PAUSE 1 #define XENSND_OP_TRIGGER_STOP 2 #define XENSND_OP_TRIGGER_RESUME3 +#define XENSND_OP_HW_PARAM_FORMAT 0 +#define XENSND_OP_HW_PARAM_RATE 1 +#define XENSND_OP_HW_PARAM_BUFFER 2 +#define XENSND_OP_HW_PARAM_PERIOD 3 +#define XENSND_OP_HW_PARAM_CHANNELS 4 + /* ** * EVENT CODES @@ -828,28 +835,127 @@ struct xensnd_trigger_req { }; /* + * Request stream configuration parameter range: request interval or + * bit mask of the supported values for the parameter given. + * + * Sound device configuration for a particular stream is a limited subset + * of the multidimensional configuration available on XenStore, for instance, + * once frame rate has been selected there is a limited supported range + * for sample rates becomes available (which might be the same set configured + * on XenStore or less). For example, selecting 96kHz sample rate may limit + * number of channels available for such configuration from 4 to 2 etc. + * Thus, each call to XENSND_OP_HW_PARAM_QUERY will reduce configuration + * space making it possible to iteratively get the final stream configuration, + * used in XENSND_OP_OPEN request. + * + * See response format for this request. + * + * 01 2 3octet + * +++++ + * | id| _HW_PARAM_QUERY|reserved| 4 + * +++++ + * | reserved | 8 + * +++++ + * | param | reserved | 12 + * +++++ + * | min or mask low 32-bit | 16 + * +++++ + * | max or mask high 32-bit | 20 + * +++++ + * | reserved | 24 + * +++++ + * | reserved | 28 + * +++++ + * | reserved | 32 + * +++++ + * + * param - uint8_t, XENSND_OP_HW_PARAM_XXX value + * + * The following parameters' payload treated as interval: + * XENSND_OP_HW_PARAM_RATE + * XENSND_OP_HW_PARAM_BUFFER + * XENSND_OP_HW_PARAM_PERIOD + * XENSND_OP_HW_PARAM_CHANNELS + * + * For interval parameters the payload of the request: + * min - uint32_t, minimum value of the parameter + * max - uint32_t, maximum value of the parameter + * + * For the following parameters their min and max values are expressed in + * frames: + * XENSND_OP_HW_PARAM_BUFFER + * XENSND_OP_HW_PARAM_PERIOD + * where frame is defined as a product of the number of channels by the + * number of octets per one sample. + * + * The following parameters' payload treated as a bit mask: + * XENSND_OP_HW_PARAM_FORMAT + * + * For mask parameters the payload of the request: + * mask - uint64_t, bit mask representing values of the parameter + */ + +struct xensnd_query_hw_param_req { +uint8_t param; +uint8_t reserved[3]; +union
[Xen-devel] [PATCH v2 0/3] sndif: add explicit back and front synchronization
From: Oleksandr Andrushchenko Hello, all! In order to provide explicit synchronization between backend and frontend the following changes are introduced in the protocol: - bump protocol version to 2 - add new ring buffer for sending asynchronous events from backend to frontend to report number of bytes played by the frontend (XENSND_EVT_CUR_POS) - introduce trigger events for playback control: start/stop/pause/resume - add "req-" prefix to event-channel and ring-ref to unify naming of the Xen event channels for requests and events Changes since v1: 1. Changed protocol version definition from string to integer, so it can easily be used in comparisons. Konrad, I have removed your r-b tag for the reason of this change. 2. In order to provide explicit stream parameter negotiation between backend and frontend the following changes are introduced in the protocol: add XENSND_OP_HW_PARAM_QUERY request to read/update configuration space for the parameter given: request passes desired parameter interval (mask) and the response to this request returns min/max interval (mask) for the parameter to be used. Parameters supported by this request/response: - format mask - sample rate interval - number of channels interval - buffer size, interval, frames - period size, interval, frames Thank you, Oleksandr Andrushchenko Oleksandr Andrushchenko (3): sndif: Introduce protocol version sndif: Add explicit back and front synchronization sndif: Add explicit back and front parameter negotiation xen/include/public/io/sndif.h | 295 -- 1 file changed, 286 insertions(+), 9 deletions(-) -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 06/27] x86/entry/64: Adapt assembly for PIE support
On Wed, 14 Mar 2018, Peter Zijlstra wrote: > On Tue, Mar 13, 2018 at 01:59:24PM -0700, Thomas Garnier wrote: > > @@ -1576,7 +1578,9 @@ first_nmi: > > addq$8, (%rsp) /* Fix up RSP */ > > pushfq /* RFLAGS */ > > pushq $__KERNEL_CS/* CS */ > > - pushq $1f /* RIP */ > > + pushq %rax/* Support Position Independent Code */ > > + leaq1f(%rip), %rax /* RIP */ > > + xchgq %rax, (%rsp)/* Restore RAX, put 1f */ > > iretq /* continues at repeat_nmi below */ > > UNWIND_HINT_IRET_REGS > > 1: > > Urgh, xchg with a memop has an implicit LOCK prefix. this_cpu_xchg uses no lock cmpxchg as a replacement to reduce latency. From linux/arch/x86/include/asm/percpu.h /* * xchg is implemented using cmpxchg without a lock prefix. xchg is * expensive due to the implied lock prefix. The processor cannot prefetch * cachelines if xchg is used. */ #define percpu_xchg_op(var, nval) \ ({ \ typeof(var) pxo_ret__; \ typeof(var) pxo_new__ = (nval); \ switch (sizeof(var)) { \ case 1: \ asm("\n\tmov "__percpu_arg(1)",%%al"\ "\n1:\tcmpxchgb %2, "__percpu_arg(1)\ "\n\tjnz 1b"\ : "=&a" (pxo_ret__), "+m" (var) \ : "q" (pxo_new__) \ : "memory");\ break; \ case 2: \ asm("\n\tmov "__percpu_arg(1)",%%ax"\ "\n1:\tcmpxchgw %2, "__percpu_arg(1)\ "\n\tjnz 1b"\ : "=&a" (pxo_ret__), "+m" (var) \ : "r" (pxo_new__) \ : "memory");\ break; \ case 4: \ asm("\n\tmov "__percpu_arg(1)",%%eax" \ "\n1:\tcmpxchgl %2, "__percpu_arg(1)\ "\n\tjnz 1b"\ : "=&a" (pxo_ret__), "+m" (var) \ : "r" (pxo_new__) \ : "memory");\ break; \ case 8: \ asm("\n\tmov "__percpu_arg(1)",%%rax" \ "\n1:\tcmpxchgq %2, "__percpu_arg(1)\ "\n\tjnz 1b"\ : "=&a" (pxo_ret__), "+m" (var) \ : "r" (pxo_new__) \ : "memory");\ break; \ default: __bad_percpu_size(); \ } \ pxo_ret__; \ ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [SVM] Getting the length of the current instruction in svm_vmexit_handler()
>>> On 14.03.18 at 15:56, wrote: > We'd like to retrieve the length of the current instruction in > svm_vmexit_handler(), specifically for the VMEXIT_EXCEPTION_DB and > VMEXIT_EXCEPTION_BP cases. > > We've combed the vmcb to no avail. Everything we've thought to check > (exitinfo1, exitinfo2, exitintinfo) turns out to be zero there while > testing. > > There's __get_instruction_length(vcpu, instr), but it expects to be fed > the exact instruction we want the length for, which obviously defeats > the purpose here. > > Is there a clean way to get the current instruction length like we do in > the VMX case (__vmread(VM_EXIT_INSTRUCTION_LEN, &insn_len)) that we're > overlooking? Just like Intel's, AMD's is available in a subset of cases only (look for vmcb->guest_ins_len), which don't include the exception intercepts you talk about. For #DB I think there's no difference between both anyway. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 120745: regressions - FAIL
flight 120745 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/120745/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-xsm 6 xen-buildfail REGR. vs. 120679 build-armhf 6 xen-buildfail REGR. vs. 120679 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass version targeted for testing: xen db0c7dde021c29c2ae0d847d70fb7b59e02ea522 baseline version: xen eef83fd2af0d4c78afec34c199c977fc97d8a0b3 Last test of basis 120679 2018-03-13 12:06:56 Z1 days Failing since120685 2018-03-13 17:01:17 Z0 days7 attempts Testing same since 120745 2018-03-14 13:41:58 Z0 days1 attempts People who touched revisions under test: Anthony PERARD Doug Goldstein Jan Beulich John Thomson Michael Young Roger Pau Monne Roger Pau Monné Wei Liu jobs: build-arm64-xsm fail build-amd64 pass build-armhf fail build-amd64-libvirt pass test-armhf-armhf-xl blocked test-arm64-arm64-xl-xsm blocked 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 Not pushing. commit db0c7dde021c29c2ae0d847d70fb7b59e02ea522 Author: Anthony PERARD Date: Tue Mar 13 11:13:18 2018 + libxl_qmp: Tell QEMU about live migration or snapshot Since version 2.10, QEMU will lock the disk images so a second QEMU instance will not try to open it. This would prevent live migration from working correctly. A new parameter as been added to the QMP command "xen-save-devices-state" in QEMU version 2.11 which allow to unlock the disk image for a live migration, but also keep it locked for a snapshot. QEMU commit: 5d6c599fe1d69a1bf8c5c4d3c58be2b31cd625ad "migration, xen: Fix block image lock issue on live migration" The extra "live" parameter can only be use if QEMU knows about it, so only add it if qemu is recent enough. The struct libxl__domain_suspend_state as now knowledge if the suspend is part of a live migration. Signed-off-by: Anthony PERARD Acked-by: Wei Liu commit ab73254b9ac3febe0c512e21af567fa536c20ad4 Author: Anthony PERARD Date: Tue Mar 13 11:13:17 2018 + libxl: Add a version check of QEMU for QMP commands On connection to QEMU via QMP, the version of QEMU is provided, store it for later use. Add a function qmp_qemu_check_version that can be used to check if QEMU is new enough for certain fonctionnality. This will be used in a moment. As it's a static function, it is commented out until first use, which is in the next patch. Signed-off-by: Anthony PERARD Acked-by: Wei Liu commit 93de1d0b3462f20da819fb4f296be8bd3271f885 Author: Wei Liu Date: Wed Mar 14 11:02:31 2018 + gitignore: ignore wrappers.c link for fuzzer At the same time reorder the entries alphabetically. Signed-off-by: Wei Liu Acked-by: Jan Beulich commit 82bc81c17d9397aad3f9386a60316cc1088a643f Author: Roger Pau Monne Date: Wed Mar 14 11:09:24 2018 + xl: remove apic option for PVH guests XSA-256 forces the local APIC to always be enabled for PVH guests, so ignore any apic option for PVH guests. Update the documentation accordingly. Signed-off-by: Roger Pau Monné Acked-by: Wei Liu commit 87554421b9ffcebe0a89ba9a927bccc0c7bed7f3 Author: John Thomson Date: Wed Mar 14 18:21:24 2018 +1000 tools: xenalyze.c fix format-truncation With gcc optimization enabled by: tools: detect appropriate debug optimization level b43501451733
Re: [Xen-devel] [PATCH v4 08/16] xen/mm: Drop the parameter mfn from populate_pt_range
Hi Jan, On 03/12/2018 06:36 AM, Jan Beulich wrote: Wei Liu 03/09/18 6:30 PM >>> On Mon, Mar 05, 2018 at 07:38:36AM -0700, Jan Beulich wrote: On 05.03.18 at 15:11, wrote: On 05/03/18 14:00, Jan Beulich wrote: On 05.03.18 at 14:43, wrote: Anyway, I don't have much knowledge on the x86 to make the modification that you suggested. So I am going to revert to _mfn(0) for x86. I'd prefer if you didn't, but well, it'll be one of us to clean it up then. I can keep as INVALID_MFN. But then either you or Andrew (or anyone x86 folks) would have to provide the patch to skip incrementing invalid MFN (if I understood correctly your request). Sigh - this should go together imo. While wrongly incrementing from zero was bad, wrongly wrapping from INVALID_MFN makes things worse. Try this patch? Looks fine; Julien, do you want to fold this in? Rather than folding this in, I am planning to add this patch at the beginning of the series. Cheers, Jan -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] docs: Fix entry for the "usbdev" option
The man for xl.cfg have the "devtype=hostdev" option, but xl only understand "type=hostdev", fix the manual to reflect actual implementation. Signed-off-by: Anthony PERARD --- docs/man/xl.cfg.pod.5.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in index 69552f8a05..2c1a6e1422 100644 --- a/docs/man/xl.cfg.pod.5.in +++ b/docs/man/xl.cfg.pod.5.in @@ -986,7 +986,7 @@ settings, from the following list: =over 4 -=item B +=item B Specifies USB device type. Currently only "hostdev" is supported. -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] xl.cfg error in usbdev, doc and code are different
On Wed, Mar 14, 2018 at 12:46:35PM +, Wei Liu wrote: > On Wed, Mar 14, 2018 at 12:36:18PM +, George Dunlap wrote: > > On 03/14/2018 11:45 AM, Wei Liu wrote: > > > Cc George and Juergen > > > > > > On Wed, Mar 14, 2018 at 11:43:46AM +, Anthony PERARD wrote: > > >> Hi, > > >> > > >> I followed `man xl.cfg` to add an usbdev property to my guest config, > > >> and xl rejected it. > > >> > > >> # xl create "usbdev=['devtype=hostdev,hostbus=1,hostaddr=2',]" ~/arch.hvm > > >> Unknown string `devtype=hostdev' in usbdev spec > > >> > > >> > > >> In xl_parse.c, the expected string seems to be "type=hostdev", not > > >> "devtype". > > >> > > >> What's the right property name? > > > > I did some archaeology, and it appears: > > > > * In response to v7 of Chunyan's pvusb series, I suggested adding the > > 'type=hostdev' option (in response to patch 7/7, xl.cfg) > > > > * In v8 of the series, Chunyan added the "devtype=hostdev" option (in > > 5/7, the command-line parsing functions, which are re-used for config > > parsing). It was also called "devtype" in the xl.cfg man page. > > > > * In v9 first send, Chunyan still had "devtype=hostdev" in the parser > > and the man page > > > > * In v9's RESEND (to which I gave my R-b), it had silently changed to > > "type=hostdev" in the parser, but was still "devtype=hostdev" in the man > > page. > > > > Personally I'd probably change the docs to fit the actual behavior. Any > > other thoughts? > > I don't have an opinion on which entity to chang if there is no > compatibility issue. "type" seems to be ignore by everything beside the xl parser. libvirt doesn't specify it, libxl doesn't check it. All assume that it's a "hostdev", which is fine because it's the only supported type. I'm also to change the doc. I'll send a patch. -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [SVM] Getting the length of the current instruction in svm_vmexit_handler()
Hello, We'd like to retrieve the length of the current instruction in svm_vmexit_handler(), specifically for the VMEXIT_EXCEPTION_DB and VMEXIT_EXCEPTION_BP cases. We've combed the vmcb to no avail. Everything we've thought to check (exitinfo1, exitinfo2, exitintinfo) turns out to be zero there while testing. There's __get_instruction_length(vcpu, instr), but it expects to be fed the exact instruction we want the length for, which obviously defeats the purpose here. Is there a clean way to get the current instruction length like we do in the VMX case (__vmread(VM_EXIT_INSTRUCTION_LEN, &insn_len)) that we're overlooking? Thanks, Razvan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v10] run QEMU as non-root
On Thu, Nov 5, 2015 at 12:47 PM, Stefano Stabellini wrote: > diff --git a/docs/misc/qemu-deprivilege.txt b/docs/misc/qemu-deprivilege.txt > new file mode 100644 > index 000..dde74ab > --- /dev/null > +++ b/docs/misc/qemu-deprivilege.txt > @@ -0,0 +1,31 @@ > +For security reasons, libxl tries to pass a non-root username to QEMU as > +argument. During initialization QEMU calls setuid and setgid with the > +user ID and the group ID of the user passed as argument. > +Libxl looks for the following users in this order: > + > +1) a user named "xen-qemuuser-domid$domid", > +Where $domid is the domid of the domain being created. > +This requires the reservation of 65535 uids from xen-qemuuser-domid1 > +to xen-qemuuser-domid65535. To use this mechanism, you might want to > +create a large number of users at installation time. For example: > + > +for ((i=1; i<65536; i++)) > +do > +adduser --no-create-home --system xen-qemuuser-domid$i > +done This fails for me after a few hundred uids: adduser: No UID is available in the range 100-999 (FIRST_SYS_UID - LAST_SYS_UID). adduser: The user `xen-qemuuser-domid892' was not created. adduser: No UID is available in the range 100-999 (FIRST_SYS_UID - LAST_SYS_UID). adduser: The user `xen-qemuuser-domid893' was not created. adduser: No UID is available in the range 100-999 (FIRST_SYS_UID - LAST_SYS_UID). adduser: The user `xen-qemuuser-domid894' was not created. It looks like even if --system were omitted, this would fail on a normal system, as the default UID range looks to be [1000,2]. Also, on my test box a single 'adduser' takes about 1 second, meaning just doing up to the normal number of domains (around 32k) would take 9 hours or so; is that really a practical suggestion? -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 39/57] ARM: new VGIC: Add ACTIVE registers handlers
Hi, On 03/14/2018 02:30 PM, Andre Przywara wrote: Hi, On 13/03/18 17:42, Julien Grall wrote: Hi, On 13/03/18 17:34, Andre Przywara wrote: On 13/03/18 17:14, Julien Grall wrote: On 13/03/18 17:02, Andre Przywara wrote: On 08/03/18 15:39, Julien Grall wrote: On 05/03/18 16:03, Andre Przywara wrote: I can't see how a knowledgeable admin will be able to know that IRQ 2 is active with just the register value. Well, I was assuming that a really knowledgeable admin would somehow forward the error message either to the ML or at least to $search_engine. ... Surely, but it does not mean the message should be clueless for the developer. I would rather no spent 10 min to try to find out what's going on where reading logs... Does that sound OK? I would still prefer the one per IRQ and using printk(XENLOG_G_*). I really don't think one per IRQ is too useful. A developer however can easily deal with "IRQ45, value: 0x00802000" from a log. And can deduce from there that it's about IRQ45 and IRQ55. Following the example above you would either see one "IRQ32, value: 0x" or "IRQ 45, value: 0x2000". I still can't see how the developer would know the IRQ55 is active or not. That's the whole purpose of the per IRQ. That looks like a good compromise between readability (having the IRQ number for admins) and brevity. You may save 10 characters on the logs, you likely going to waste 10 min of the developer to understand what that messages really mean. It's not about 10 characters, it's about 31 *lines*. At the moment a write to I[CS]ACTIVER triggers *one* line in the log: %pv: vGICD: unhandled word write %#"PRIregister" to ISACTIVER%d Now a write to this register would potentially trigger *32* lines: %pv: vGICD: IRQ%u: setting active state not supported Very likely there will 0 lines printed because clearing an active interrupt should never happen in Xen guest today. So if there is 32 lines printed, then having 32 lines in the log is your last of your concern. By dumping the line as I proposed, I basically mimic the current line, plus give some information about one IRQ affected: %pv: vGICD: setting active state not supported (IRQ%u, value: 0x%08lx) So this is not a regression, but an improvement. I never said it was a regression. I said your new message and bail out is counter-intuitive because the developer can't guess if there are other IRQs active with just "value". If you want to make an improvement, do it properly. In that case, I am only asking to drop the counter-intuitive bail_out. Cheers. -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2] libxl: allow libxl_domain_suspend to simply suspend a domain, without saving it
When LIBXL_SUSPEND_NO_SAVE flag is set, no savefile will be written, but the domain will still be suspended (but not destroyed). The main reason for this functionality is to suspend the host while some domains are running, potentially holding PCI devices. This will give a chance to a driver in such a domain to properly suspend the device. It would be better to have a separate function for this, but in fact it should be named libxl_domain_suspend, then the current one renamed to libxl_domain_save. Since that would break API compatibility, keep it in the same function. Signed-off-by: Marek Marczykowski-Górecki Signed-off-by: Marcus of Wetware Labs --- Changes in v2: - drop double initialization of dsps fields (libxl__domain_suspend_init is called) - use LIBXL_SUSPEND_NO_SAVE flag instead of fd=-1 --- tools/libxl/libxl.h| 5 + tools/libxl/libxl_domain.c | 52 +- 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h index eca0ea2c50..636db77c2b 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -1469,6 +1469,11 @@ int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd, LIBXL_EXTERNAL_CALLERS_ONLY; #define LIBXL_SUSPEND_DEBUG 1 #define LIBXL_SUSPEND_LIVE 2 +/* + * Just transition the domain into suspended state, do not save its state to + * disk and do not destroy it. fd parameter is ignored. + */ +#define LIBXL_SUSPEND_NO_SAVE 4 /* @param suspend_cancel [from xenctrl.h:xc_domain_resume( @param fast )] * If this parameter is true, use co-operative resume. The guest diff --git a/tools/libxl/libxl_domain.c b/tools/libxl/libxl_domain.c index 13b1c73d40..0e9e245ce3 100644 --- a/tools/libxl/libxl_domain.c +++ b/tools/libxl/libxl_domain.c @@ -486,6 +486,13 @@ static void domain_suspend_cb(libxl__egc *egc, } +static void domain_suspend_empty_cb(libxl__egc *egc, + libxl__domain_suspend_state *dss, int rc) +{ +STATE_AO_GC(dss->ao); +libxl__ao_complete(egc,ao,rc); +} + int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd, int flags, const libxl_asyncop_how *ao_how) { @@ -498,25 +505,40 @@ int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd, int flags, goto out_err; } -libxl__domain_save_state *dss; -GCNEW(dss); +if (!(flags & LIBXL_SUSPEND_NO_SAVE)) { +libxl__domain_save_state *dss; -dss->ao = ao; -dss->callback = domain_suspend_cb; +GCNEW(dss); -dss->domid = domid; -dss->fd = fd; -dss->type = type; -dss->live = flags & LIBXL_SUSPEND_LIVE; -dss->debug = flags & LIBXL_SUSPEND_DEBUG; -dss->checkpointed_stream = LIBXL_CHECKPOINTED_STREAM_NONE; +dss->ao = ao; +dss->callback = domain_suspend_cb; + +dss->domid = domid; +dss->fd = fd; +dss->type = type; +dss->live = flags & LIBXL_SUSPEND_LIVE; +dss->debug = flags & LIBXL_SUSPEND_DEBUG; +dss->checkpointed_stream = LIBXL_CHECKPOINTED_STREAM_NONE; + +rc = libxl__fd_flags_modify_save(gc, dss->fd, + ~(O_NONBLOCK|O_NDELAY), 0, + &dss->fdfl); +if (rc < 0) goto out_err; -rc = libxl__fd_flags_modify_save(gc, dss->fd, - ~(O_NONBLOCK|O_NDELAY), 0, - &dss->fdfl); -if (rc < 0) goto out_err; +libxl__domain_save(egc, dss); +} else { +libxl__domain_suspend_state *dsps; + +GCNEW(dsps); +dsps->ao = ao; +dsps->domid = domid; +dsps->type = type; +rc = libxl__domain_suspend_init(egc, dsps, type); +if (rc < 0) goto out_err; +dsps->callback_common_done = domain_suspend_empty_cb; +libxl__domain_suspend(egc, dsps); +} -libxl__domain_save(egc, dss); return AO_INPROGRESS; out_err: -- 2.13.6 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 39/57] ARM: new VGIC: Add ACTIVE registers handlers
Hi, On 13/03/18 17:42, Julien Grall wrote: > Hi, > > On 13/03/18 17:34, Andre Przywara wrote: >> On 13/03/18 17:14, Julien Grall wrote: >>> On 13/03/18 17:02, Andre Przywara wrote: On 08/03/18 15:39, Julien Grall wrote: > On 05/03/18 16:03, Andre Przywara wrote: >>> I can't see how a knowledgeable admin will be able to know that IRQ 2 is >>> active with just the register value. >> >> Well, I was assuming that a really knowledgeable admin would somehow >> forward the error message either to the ML or at least to $search_engine. >> ... > > Surely, but it does not mean the message should be clueless for the > developer. I would rather no spent 10 min to try to find out what's > going on where reading logs... > >> Does that sound OK? >>> >>> I would still prefer the one per IRQ and using printk(XENLOG_G_*). >> >> I really don't think one per IRQ is too useful. A developer however can >> easily deal with "IRQ45, value: 0x00802000" from a log. And can deduce >> from there that it's about IRQ45 and IRQ55. Following the example above >> you would either see one "IRQ32, value: 0x" or "IRQ 45, value: >> 0x2000". > > I still can't see how the developer would know the IRQ55 is active or > not. That's the whole purpose of the per IRQ. > >> That looks like a good compromise between readability (having the IRQ >> number for admins) and brevity. > > You may save 10 characters on the logs, you likely going to waste 10 min > of the developer to understand what that messages really mean. It's not about 10 characters, it's about 31 *lines*. At the moment a write to I[CS]ACTIVER triggers *one* line in the log: %pv: vGICD: unhandled word write %#"PRIregister" to ISACTIVER%d Now a write to this register would potentially trigger *32* lines: %pv: vGICD: IRQ%u: setting active state not supported By dumping the line as I proposed, I basically mimic the current line, plus give some information about one IRQ affected: %pv: vGICD: setting active state not supported (IRQ%u, value: 0x%08lx) So this is not a regression, but an improvement. Cheers, Andre. >> I changed it now to output: >> %pv: vGICD: clearing active state not supported (IRQ%u, value: 0x%08lx) >> >>> I don't much care about the spam, see why above. >> >> Having them on the console between Dom0 messages is really scary, but >> not helpful if it's *more* than one. Since it's a known limitation of >> the VGIC emulation, not a real "error" in that sense. > > It is the same as having any Xen messages interleaved with Dom0 > messages. If the user is not happy with that, then it can divert Dom0 > console to another UART. > > Cheers, > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v9 01/11] vpci: introduce basic handlers to trap accesses to the PCI config space
>>> On 14.03.18 at 15:03, wrote: > --- a/xen/drivers/Kconfig > +++ b/xen/drivers/Kconfig > @@ -12,4 +12,6 @@ source "drivers/pci/Kconfig" > > source "drivers/video/Kconfig" > > +source "drivers/vpci/Kconfig" Are there more things to appear in that new file? If not, what the point of introducing it? > --- /dev/null > +++ b/xen/drivers/vpci/Kconfig > @@ -0,0 +1,4 @@ > + > +config HAS_VPCI > + bool > + depends on HAS_PCI So this is one of those common Kconfig issues: When there's no prompt, any "depends on" can at best lead to warnings the tool issues and no-one pays attention to. I can only advise to avoid such dependencies. If anyone gets the selects wrong, there'll be a build failure somewhere independent of said warning. With these taken care of, feel free to reinstate Reviewed-by: Jan Beulich (I think the adjustments could also be done while committing, if enough of the series becomes ready to warrant putting in at least part of it; I think it was really the lack of an ARM side ack which prevented me from doing this on v8 already.) Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] Deprecated option -usbdevice in QEMU
Hi, In an xl guest config, we have the "usbdevice" option. It is just passthrough to QEMU "-usbdevice" without parsing. The QEMU option is now deprecated. v2.11 (to be released with Xen 4.11) is the last version of QEMU to have the option. Unfortunatly, our documentation relie on QEMU's documentation, so there would be a lot to parse if we want to keep the option. I propose that we also deprecated the "usbdevice" option and find a suitable alternative (or we have to parse usbdevice in libxl). "usbdev" seems to be a good fit for that, and should already handle "usbdevice='host:bus.addr', but would be written: "usbdev=['type=hostdev,hostbus=bus,hostaddr=addr']" The other use of "usbdevice" documented on the man are: - tablet - host:vendor_id:product_id Other usage of "usbdevice" documented in the QEMU documentation: - mouse - disk:[format=format]:file - serial:[vendorid=vendor_id][,productid=product_id]:dev - braille - net:options From QEMU perspective, those options can be replaced the following cmdline options or QMP command equivalent (I haven't check everything): * -device usb-tablet * -device usb-host,vendorid=vendor,productid=product * -device usb-mouse * -drive if=none,id=drive_id,file=file -device usb-storage,drive=drive_id * -chardev x,id=id -device usb-serial,chardev=id * -device usb-braille * -netdev x,id=id -device usb-net,netdev=id How the original "usbdevice" could be translated into a USBDEV_SPEC for "usbdev" ? Maybe for e.g.: 'type=tablet' 'type=storage,file=file,format=format' I don't know is anybody would be using "usbdevice='net:...'" or "usbdevice='serial:...'". What do you think? Thanks, -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC PATCH 0/6] Using GitLab CI for build testing
On 03/14/2018 01:28 PM, Doug Goldstein wrote: > On 3/14/18 4:45 AM, George Dunlap wrote: >> On Tue, Mar 13, 2018 at 3:31 AM, Doug Goldstein wrote: >>> Really early work on switching over to using GitLab CI over >>> Travis CI. GitLab is a competitor to GitHub with some advantages >>> such as an integrated CI system with a lot more flexibility >>> and control. It additionally is fully open sourced unlike GitHub >>> and Travis CI. We can even run an instance if that is preferred >>> over using the hosted instance. >>> >>> This change uses GitLab CI's ability to use Docker based runners >>> for running tests. With GitHub we also use a Docker based runner >>> but we are limited to one Docker container that is then morphed >>> a number of different ways. With this approach we can specify >>> different Docker containers for every run (or use the same). By >>> using different Docker containers we can build environments that >>> match systems where Xen can and should build. Using this >>> approach we should be able to cutdown on the number of surpise >>> build failures encountered by users. >>> >>> An example run can be seen here: >>> https://gitlab.com/cardoe/xen/pipelines/18789907 >>> >>> If there is interest in this I will move it over to the "xen-project" >>> name space in the next version. >>> >>> Doug Goldstein (6): >>> ci: add Dockerfile for CentOS 7.2 >>> ci: add Dockerfile for Ubuntu 14.04 >>> ci: add Dockerfile for Ubuntu 16.04 >>> ci: add Dockerfile for Debian jessie >>> ci: add cfg to use GitLab CI to build >>> ci: add a README about the containers >>> >>> .gitlab-ci.yml | 34 ++- >>> extras/testing/README.md| 29 ++- >>> extras/testing/centos/CentOS-7.2.repo | 35 ++- >>> extras/testing/centos/Dockerfile.7.2| 41 ++- >>> extras/testing/debian/Dockerfile.jessie | 21 +- >>> extras/testing/ubuntu/Dockerfile.trusty | 21 +- >>> extras/testing/ubuntu/Dockerfile.xenial | 21 +- >> >> "extras" is a bit generic. What about something like "automation/build"? >> >> (You knew this bike shed wasn't going to get in without *some* >> discussion of the color!) >> >> -George >> > > Ha. Thank you. So in the same thread I'll move the helper script that > the CI will use into "automation/scripts"? Currently we have > "scripts/travis-build" but there are going to be some more (specifically > for ARM) Yes, we should try to group all the "hooks for external automated tools" together somehow. FWIW, although I still prefer "automation", upon reflection "extras" is a nice color too. :-) Thanks again for doing this. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-4.7-testing test] 120594: regressions - FAIL
flight 120594 xen-4.7-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/120594/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemut-ws16-amd64 10 windows-install fail REGR. vs. 119780 Tests which are failing intermittently (not blocking): test-xtf-amd64-amd64-2 50 xtf/test-hvm64-lbr-tsx-vmentry fail in 120425 pass in 120594 test-xtf-amd64-amd64-5 50 xtf/test-hvm64-lbr-tsx-vmentry fail pass in 120425 test-xtf-amd64-amd64-3 50 xtf/test-hvm64-lbr-tsx-vmentry fail pass in 120425 Tests which did not succeed, but are not blocking: test-armhf-armhf-xl-rtds 17 guest-start.2 fail blocked in 119780 test-xtf-amd64-amd64-4 50 xtf/test-hvm64-lbr-tsx-vmentry fail in 120425 like 119780 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeat fail in 120425 like 119780 test-xtf-amd64-amd64-1 50 xtf/test-hvm64-lbr-tsx-vmentry fail like 119780 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail like 119780 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 119780 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 119780 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 119780 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 119780 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 119780 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 119780 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 119780 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 119780 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 119780 test-xtf-amd64-amd64-5 52 xtf/test-hvm64-memop-seg fail never pass test-xtf-amd64-amd64-2 52 xtf/test-hvm64-memop-seg fail never pass test-xtf-amd64-amd64-3 52 xtf/test-hvm64-memop-seg fail never pass test-xtf-amd64-amd64-4 52 xtf/test-hvm64-memop-seg fail never pass test-xtf-amd64-amd64-1 52 xtf/test-hvm64-memop-seg fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-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-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass te
Re: [Xen-devel] [PATCH v4] tools: detect appropriate debug optimization level
On Mon, Mar 12, 2018 at 11:06:51PM -0500, Doug Goldstein wrote: > When building debug use -Og as the optimization level if its available, > otherwise retain the use of -O0. -Og has been added by GCC to enable all > optimizations that to not affect debugging while retaining full > debugability. > > Signed-off-by: Doug Goldstein > --- > tools/Rules.mk | 9 +++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/tools/Rules.mk b/tools/Rules.mk > index 296b722372..3848bcf1f7 100644 > --- a/tools/Rules.mk > +++ b/tools/Rules.mk > @@ -164,8 +164,13 @@ LDLIBS_libxenvchan = $(SHDEPS_libxenvchan) > $(XEN_LIBVCHAN)/libxenvchan$(libexten > SHLIB_libxenvchan = $(SHDEPS_libxenvchan) -Wl,-rpath-link=$(XEN_LIBVCHAN) > > ifeq ($(debug),y) > -# Disable optimizations > -CFLAGS += -O0 -fno-omit-frame-pointer > +CFLAGS += -fno-omit-frame-pointer > +# Use optimizations compatible with debugging otherwise disable optimizations > +ifneq ($(call cc-option,$(CC),-Og,n),n) > +CFLAGS += -Og > +else > +CFLAGS += -O0 > +endif Sadly the way cc-option is written made the support check always return true. cc-option would grep for -Og in the (error) to tell if it is supported or not, but the error message for -Og doesn't contain "-Og". It is like cc1: error: argument to '-O' should be a non-negative integer We need to think of another way to test it, or we just have to live with V1. Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4] tools: detect appropriate debug optimization level
On Wed, Mar 14, 2018 at 02:16:31PM +, Wei Liu wrote: > On Mon, Mar 12, 2018 at 11:06:51PM -0500, Doug Goldstein wrote: > > When building debug use -Og as the optimization level if its available, > > otherwise retain the use of -O0. -Og has been added by GCC to enable all > > optimizations that to not affect debugging while retaining full > > debugability. > > > > Signed-off-by: Doug Goldstein > > --- > > tools/Rules.mk | 9 +++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/tools/Rules.mk b/tools/Rules.mk > > index 296b722372..3848bcf1f7 100644 > > --- a/tools/Rules.mk > > +++ b/tools/Rules.mk > > @@ -164,8 +164,13 @@ LDLIBS_libxenvchan = $(SHDEPS_libxenvchan) > > $(XEN_LIBVCHAN)/libxenvchan$(libexten > > SHLIB_libxenvchan = $(SHDEPS_libxenvchan) -Wl,-rpath-link=$(XEN_LIBVCHAN) > > > > ifeq ($(debug),y) > > -# Disable optimizations > > -CFLAGS += -O0 -fno-omit-frame-pointer > > +CFLAGS += -fno-omit-frame-pointer > > +# Use optimizations compatible with debugging otherwise disable > > optimizations > > +ifneq ($(call cc-option,$(CC),-Og,n),n) > > +CFLAGS += -Og > > +else > > +CFLAGS += -O0 > > +endif > > Sadly the way cc-option is written made the support check always > return true. cc-option would grep for -Og in the (error) to tell if > it is supported or not, but the error message for -Og doesn't contain > "-Og". It is like > > cc1: error: argument to '-O' should be a non-negative integer > > We need to think of another way to test it, or we just have to live with > V1. Wait, but even v1 is bogus because cc-option-add uses cc-option. I'm afraid we will need to figure out a new way to detect if the option is supported. Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v9 06/11] xen: introduce rangeset_consume_ranges
This function allows to iterate over a rangeset while removing the processed regions. This will be used in order to split processing of large memory areas when mapping them into the guest p2m. Signed-off-by: Roger Pau Monné Reviewed-by: Wei Liu --- 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 --- Changes since v6: - Expand commit message. - Add a comment to describe the expected function behavior. - Fix indentation. Changes since v5: - New in this version. --- xen/common/rangeset.c | 28 xen/include/xen/rangeset.h | 10 ++ 2 files changed, 38 insertions(+) diff --git a/xen/common/rangeset.c b/xen/common/rangeset.c index ade34f6a50..bb68ce62e4 100644 --- a/xen/common/rangeset.c +++ b/xen/common/rangeset.c @@ -350,6 +350,34 @@ int rangeset_claim_range(struct rangeset *r, unsigned long size, return 0; } +int rangeset_consume_ranges(struct rangeset *r, +int (*cb)(unsigned long s, unsigned long e, void *, + unsigned long *c), +void *ctxt) +{ +int rc = 0; + +write_lock(&r->lock); +while ( !rangeset_is_empty(r) ) +{ +unsigned long consumed = 0; +struct range *x = first_range(r); + +rc = cb(x->s, x->e, ctxt, &consumed); + +ASSERT(consumed <= x->e - x->s + 1); +x->s += consumed; +if ( x->s > x->e ) +destroy_range(r, x); + +if ( rc ) +break; +} +write_unlock(&r->lock); + +return rc; +} + int rangeset_add_singleton( struct rangeset *r, unsigned long s) { diff --git a/xen/include/xen/rangeset.h b/xen/include/xen/rangeset.h index 1f83b1f44b..583b72bb0c 100644 --- a/xen/include/xen/rangeset.h +++ b/xen/include/xen/rangeset.h @@ -70,6 +70,16 @@ int rangeset_report_ranges( struct rangeset *r, unsigned long s, unsigned long e, int (*cb)(unsigned long s, unsigned long e, void *), void *ctxt); +/* + * Note that the consume function can return an error value apart from + * -ERESTART, and that no cleanup is performed (ie: the user should call + * rangeset_destroy if needed). + */ +int rangeset_consume_ranges(struct rangeset *r, +int (*cb)(unsigned long s, unsigned long e, + void *, unsigned long *c), +void *ctxt); + /* Add/remove/query a single number. */ int __must_check rangeset_add_singleton( struct rangeset *r, unsigned long s); -- 2.16.2 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v9 02/11] x86/mmcfg: add handlers for the PVH Dom0 MMCFG areas
Introduce a set of handlers for the accesses to the MMCFG areas. Those areas are setup based on the contents of the hardware MMCFG tables, and the list of handled MMCFG areas is stored inside of the hvm_domain struct. The read/writes are forwarded to the generic vpci handlers once the address is decoded in order to obtain the device and register the guest is trying to access. Signed-off-by: Roger Pau Monné Reviewed-by: Paul Durrant Reviewed-by: Jan Beulich --- Cc: Jan Beulich Cc: Andrew Cooper Cc: Paul Durrant --- Changes since v7: - Add check for end_bus >= start_bus to register_vpci_mmcfg_handler. - Protect destroy_vpci_mmcfg with the mmcfg_lock. Changes since v6: - Move allocation of mmcfg outside of the locked region. - Do proper overlap checks when adding mmcfg regions. - Return _RETRY if the mcfg region cannot be found in the read/write handlers. This means the mcfg area has been removed between the accept and the read/write calls. Changes since v5: - Switch to use pci_sbdf_t. - Switch to the new per vpci locks. - Move the mmcfg related external definitions to asm-x86/pci.h. Changes since v4: - Change the attribute of pvh_setup_mmcfg to __hwdom_init. - Try to add as many MMCFG regions as possible, even if one fails to add. - Change some fields of the hvm_mmcfg struct: turn size into a unsigned int, segment into uint16_t and bus into uint8_t. - Convert some address parameters from unsigned long to paddr_t for consistency. - Make vpci_mmcfg_decode_addr return the decoded register in the return of the function. - Introduce a new macro to convert a MMCFG address into a BDF, and use it in vpci_mmcfg_decode_addr to clarify the logic. - In vpci_mmcfg_{read/write} unify the logic for 8B accesses and smaller ones. - Add the __hwdom_init attribute to register_vpci_mmcfg_handler. - Test that reg + size doesn't cross a device boundary. Changes since v3: - Propagate changes from previous patches: drop xen_ prefix for vpci functions, pass slot and func instead of devfn and fix the error paths of the MMCFG handlers. - s/ecam/mmcfg/. - Move the destroy code to a separate function, so the hvm_mmcfg struct can be private to hvm/io.c. - Constify the return of vpci_mmcfg_find. - Use d instead of v->domain in vpci_mmcfg_accept. - Allow 8byte accesses to the mmcfg. Changes since v1: - Added locking. --- xen/arch/x86/hvm/dom0_build.c| 21 + xen/arch/x86/hvm/hvm.c | 4 + xen/arch/x86/hvm/io.c| 184 ++- xen/arch/x86/x86_64/mmconfig.h | 4 - xen/include/asm-x86/hvm/domain.h | 4 + xen/include/asm-x86/hvm/io.h | 7 ++ xen/include/asm-x86/pci.h| 6 ++ 7 files changed, 225 insertions(+), 5 deletions(-) diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c index afebaec70b..b8aa132adf 100644 --- a/xen/arch/x86/hvm/dom0_build.c +++ b/xen/arch/x86/hvm/dom0_build.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -1055,6 +1056,24 @@ static int __init pvh_setup_acpi(struct domain *d, paddr_t start_info) return 0; } +static void __hwdom_init pvh_setup_mmcfg(struct domain *d) +{ +unsigned int i; +int rc; + +for ( i = 0; i < pci_mmcfg_config_num; i++ ) +{ +rc = register_vpci_mmcfg_handler(d, pci_mmcfg_config[i].address, + pci_mmcfg_config[i].start_bus_number, + pci_mmcfg_config[i].end_bus_number, + pci_mmcfg_config[i].pci_segment); +if ( rc ) +printk("Unable to setup MMCFG handler at %#lx for segment %u\n", + pci_mmcfg_config[i].address, + pci_mmcfg_config[i].pci_segment); +} +} + int __init dom0_construct_pvh(struct domain *d, const module_t *image, unsigned long image_headroom, module_t *initrd, @@ -1096,6 +1115,8 @@ int __init dom0_construct_pvh(struct domain *d, const module_t *image, return rc; } +pvh_setup_mmcfg(d); + panic("Building a PVHv2 Dom0 is not yet supported."); return 0; } diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index a840130c17..690d68aefe 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -585,8 +585,10 @@ int hvm_domain_initialise(struct domain *d, unsigned long domcr_flags, spin_lock_init(&d->arch.hvm_domain.irq_lock); spin_lock_init(&d->arch.hvm_domain.uc_lock); spin_lock_init(&d->arch.hvm_domain.write_map.lock); +rwlock_init(&d->arch.hvm_domain.mmcfg_lock); INIT_LIST_HEAD(&d->arch.hvm_domain.write_map.list); INIT_LIST_HEAD(&d->arch.hvm_domain.g2m_ioport_list); +INIT_LIST_HEAD(&d->arch.hvm_domain.mmcfg_regions); rc = create_perdomain_mapping(d, PERDOMAIN_VIRT_START, 0, NULL, NULL); if ( rc ) @@ -732,6 +734,8 @@
[Xen-devel] [PATCH v9 00/11] vpci: PCI config space emulation
Hello, The following series contain an implementation of handlers for the PCI configuration space inside of Xen. This allows Xen to detect accesses to the PCI configuration space and react accordingly. Why is this needed? IMHO, there are two main points of doing all this emulation inside of Xen, the first one is to prevent adding a bunch of duplicated Xen PV specific code to each OS we want to support in PVH mode. This just promotes Xen code duplication amongst OSes, which leads to a higher maintainership burden. The second reason would be that this code (or it's functionality to be more precise) already exists in QEMU (and pciback to a degree), and it's code that we already support and maintain. By moving it into the hypervisor itself every guest type can make use of it, and should be shared between them all. I know that the code in this series is not yet suitable for DomU HVM guests in it's current state, but it should be in due time. As usual, each patch contains a changeset summary between versions, I'm not going to copy the list of changes here. The branch containing the patches can be found at: git://xenbits.xen.org/people/royger/xen.git vpci_v9 Note that this is only safe to use for the hardware domain (that's trusted), any non-trusted domain will need a lot more handlers before it can freely access the PCI configuration space. Roger Pau Monne (11): vpci: introduce basic handlers to trap accesses to the PCI config space x86/mmcfg: add handlers for the PVH Dom0 MMCFG areas x86/physdev: enable PHYSDEVOP_pci_mmcfg_reserved for PVH Dom0 pci: split code to size BARs from pci_add_device pci: add support to size ROM BARs to pci_size_mem_bar xen: introduce rangeset_consume_ranges vpci/bars: add handlers to map the BARs x86/pt: mask MSI vectors on unbind vpci/msi: add MSI handlers vpci: add a priority parameter to the vPCI register initializer vpci/msix: add MSI-X handlers .gitignore| 3 + tools/libxl/libxl_x86.c | 2 +- tools/tests/Makefile | 1 + tools/tests/vpci/Makefile | 37 +++ tools/tests/vpci/emul.h | 134 + tools/tests/vpci/main.c | 309 + xen/arch/arm/xen.lds.S| 14 + xen/arch/x86/Kconfig | 1 + xen/arch/x86/domain.c | 6 +- xen/arch/x86/hvm/dom0_build.c | 23 +- xen/arch/x86/hvm/hvm.c| 7 + xen/arch/x86/hvm/hypercall.c | 5 + xen/arch/x86/hvm/io.c | 293 xen/arch/x86/hvm/ioreq.c | 4 + xen/arch/x86/hvm/vmsi.c | 231 xen/arch/x86/msi.c| 3 + xen/arch/x86/physdev.c| 11 + xen/arch/x86/setup.c | 3 +- xen/arch/x86/x86_64/mmconfig.h| 4 - xen/arch/x86/xen.lds.S| 14 + xen/common/rangeset.c | 28 ++ xen/drivers/Kconfig | 2 + xen/drivers/Makefile | 1 + xen/drivers/passthrough/io.c | 15 + xen/drivers/passthrough/pci.c | 107 +--- xen/drivers/vpci/Kconfig | 4 + xen/drivers/vpci/Makefile | 1 + xen/drivers/vpci/header.c | 563 ++ xen/drivers/vpci/msi.c| 335 +++ xen/drivers/vpci/msix.c | 458 +++ xen/drivers/vpci/vpci.c | 483 xen/include/asm-x86/domain.h | 1 + xen/include/asm-x86/hvm/domain.h | 7 + xen/include/asm-x86/hvm/io.h | 20 ++ xen/include/asm-x86/msi.h | 3 + xen/include/asm-x86/pci.h | 6 + xen/include/public/arch-x86/xen.h | 5 +- xen/include/xen/irq.h | 1 + xen/include/xen/pci.h | 9 + xen/include/xen/pci_regs.h| 8 + xen/include/xen/rangeset.h| 10 + xen/include/xen/sched.h | 4 + xen/include/xen/vpci.h| 226 +++ 43 files changed, 3356 insertions(+), 46 deletions(-) create mode 100644 tools/tests/vpci/Makefile create mode 100644 tools/tests/vpci/emul.h create mode 100644 tools/tests/vpci/main.c create mode 100644 xen/drivers/vpci/Kconfig create mode 100644 xen/drivers/vpci/Makefile create mode 100644 xen/drivers/vpci/header.c create mode 100644 xen/drivers/vpci/msi.c create mode 100644 xen/drivers/vpci/msix.c create mode 100644 xen/drivers/vpci/vpci.c create mode 100644 xen/include/xen/vpci.h -- 2.16.2 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v9 08/11] x86/pt: mask MSI vectors on unbind
When a MSI device with per-vector masking capabilities is detected or added to Xen all the vectors are masked when initializing it. This implies that the first time the interrupt is bound to a domain it's masked. This however only applies to the first time the interrupt is bound because neither the unbind nor the pirq unmap will mask the vector again. In order to fix this re-mask the interrupt when unbinding it from a guest. This makes sure that pairs of bind/unbind will always get the same masking state. Note that no issues have been reported regarding this behavior because QEMU always uses the newly introduced XEN_PT_GFLAGSSHIFT_UNMASKED when binding interrupts, so it's always unmasked. Signed-off-by: Roger Pau Monné Reviewed-by: Jan Beulich --- Cc: Jan Beulich --- Changes since v7: - New in this version. --- xen/drivers/passthrough/io.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/io.c index 8f16e6c0a5..bab3aa349a 100644 --- a/xen/drivers/passthrough/io.c +++ b/xen/drivers/passthrough/io.c @@ -645,7 +645,22 @@ int pt_irq_destroy_bind( } break; case PT_IRQ_TYPE_MSI: +{ +unsigned long flags; +struct irq_desc *desc = domain_spin_lock_irq_desc(d, machine_gsi, + &flags); + +if ( !desc ) +return -EINVAL; +/* + * Leave the MSI masked, so that the state when calling + * pt_irq_create_bind is consistent across bind/unbinds. + */ +guest_mask_msi_irq(desc, true); +spin_unlock_irqrestore(&desc->lock, flags); break; +} + default: return -EOPNOTSUPP; } -- 2.16.2 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v9 04/11] pci: split code to size BARs from pci_add_device
So that it can be called from outside in order to get the size of regular PCI BARs. This will be required in order to map the BARs from PCI devices into PVH Dom0 p2m. Signed-off-by: Roger Pau Monné Reviewed-by: Jan Beulich --- Cc: Jan Beulich Cc: Andrew Cooper Cc: George Dunlap Cc: Ian Jackson Cc: Julien Grall Cc: Konrad Rzeszutek Wilk Cc: Stefano Stabellini Cc: Tim Deegan Cc: Wei Liu --- Changes since v7: - Do not return error from pci_size_mem_bar in order to keep previous behavior. Changes since v6: - Remove the vf and addr local variables. - Change the way flags are declared. - Move the last bool parameter to the flags field. Changes since v5: - Introduce a flags field for pci_size_mem_bar. - Use pci_sbdf_t. Changes since v4: - Restore printing whether the BAR is from a vf. - Make the psize pointer parameter not optional. - s/u64/uint64_t. - Remove some unneeded parentheses. - Assert the return value is never 0. - Use the newly introduced pci_sbdf_t type. Changes since v3: - Rename function to size BARs to pci_size_mem_bar. - Change the parameters passed to the function. Pass the position and whether the BAR is the last one, instead of the (base, max_bars, *index) tuple. - Make the function return the number of BARs consumed (1 for 32b, 2 for 64b BARs). - Change the dprintk back to printk. - Do not log another error message in pci_add_device in case pci_size_mem_bar fails. --- xen/drivers/passthrough/pci.c | 97 --- xen/include/xen/pci.h | 5 +++ 2 files changed, 68 insertions(+), 34 deletions(-) diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c index e65c7faa6f..190515b3c6 100644 --- a/xen/drivers/passthrough/pci.c +++ b/xen/drivers/passthrough/pci.c @@ -603,6 +603,56 @@ static int iommu_add_device(struct pci_dev *pdev); static int iommu_enable_device(struct pci_dev *pdev); static int iommu_remove_device(struct pci_dev *pdev); +unsigned int pci_size_mem_bar(pci_sbdf_t sbdf, unsigned int pos, + uint64_t *paddr, uint64_t *psize, + unsigned int flags) +{ +uint32_t hi = 0, bar = pci_conf_read32(sbdf.seg, sbdf.bus, sbdf.dev, + sbdf.func, pos); +uint64_t size; + +ASSERT((bar & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_MEMORY); +pci_conf_write32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.func, pos, ~0); +if ( (bar & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == + PCI_BASE_ADDRESS_MEM_TYPE_64 ) +{ +if ( flags & PCI_BAR_LAST ) +{ +printk(XENLOG_WARNING + "%sdevice %04x:%02x:%02x.%u with 64-bit %sBAR in last slot\n", + (flags & PCI_BAR_VF) ? "SR-IOV " : "", sbdf.seg, sbdf.bus, + sbdf.dev, sbdf.func, (flags & PCI_BAR_VF) ? "vf " : ""); +*psize = 0; +return 1; +} +hi = pci_conf_read32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.func, pos + 4); +pci_conf_write32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.func, pos + 4, ~0); +} +size = pci_conf_read32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.func, pos) & + PCI_BASE_ADDRESS_MEM_MASK; +if ( (bar & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == + PCI_BASE_ADDRESS_MEM_TYPE_64 ) +{ +size |= (uint64_t)pci_conf_read32(sbdf.seg, sbdf.bus, sbdf.dev, + sbdf.func, pos + 4) << 32; +pci_conf_write32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.func, pos + 4, hi); +} +else if ( size ) +size |= (uint64_t)~0 << 32; +pci_conf_write32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.func, pos, bar); +size = -size; + +if ( paddr ) +*paddr = (bar & PCI_BASE_ADDRESS_MEM_MASK) | ((uint64_t)hi << 32); +*psize = size; + +if ( (bar & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == + PCI_BASE_ADDRESS_MEM_TYPE_64 ) +return 2; + +return 1; +} + int pci_add_device(u16 seg, u8 bus, u8 devfn, const struct pci_dev_info *info, nodeid_t node) { @@ -672,11 +722,16 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn, unsigned int i; BUILD_BUG_ON(ARRAY_SIZE(pdev->vf_rlen) != PCI_SRIOV_NUM_BARS); -for ( i = 0; i < PCI_SRIOV_NUM_BARS; ++i ) +for ( i = 0; i < PCI_SRIOV_NUM_BARS; ) { unsigned int idx = pos + PCI_SRIOV_BAR + i * 4; u32 bar = pci_conf_read32(seg, bus, slot, func, idx); -u32 hi = 0; +pci_sbdf_t sbdf = { +.seg = seg, +.bus = bus, +.dev = slot, +.func = func, +}; if ( (bar & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_IO ) @@ -687,38 +742,12 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn, seg, bus, slot, func, i);
[Xen-devel] [PATCH v9 10/11] vpci: add a priority parameter to the vPCI register initializer
This is needed for MSI-X, since MSI-X will need to be initialized before parsing the BARs, so that the header BAR handlers are aware of the MSI-X related holes and make sure they are not mapped in order for the trap handlers to work properly. Signed-off-by: Roger Pau Monné Reviewed-by: Jan Beulich --- Cc: Stefano Stabellini Cc: Julien Grall Cc: Andrew Cooper Cc: George Dunlap Cc: Ian Jackson Cc: Jan Beulich Cc: Konrad Rzeszutek Wilk Cc: Tim Deegan Cc: Wei Liu --- Changes since v4: - Add a middle priority and add the PCI header to it. Changes since v3: - Add a numerial suffix to the section used to store the pointer to each initializer function, and sort them at link time. --- xen/arch/arm/xen.lds.S| 4 ++-- xen/arch/x86/xen.lds.S| 4 ++-- xen/drivers/vpci/header.c | 2 +- xen/drivers/vpci/msi.c| 2 +- xen/include/xen/vpci.h| 8 ++-- 5 files changed, 12 insertions(+), 8 deletions(-) diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S index 49cae2af71..245a0e0e85 100644 --- a/xen/arch/arm/xen.lds.S +++ b/xen/arch/arm/xen.lds.S @@ -69,7 +69,7 @@ SECTIONS #if defined(CONFIG_HAS_VPCI) && defined(CONFIG_LATE_HWDOM) . = ALIGN(POINTER_ALIGN); __start_vpci_array = .; - *(.data.vpci) + *(SORT(.data.vpci.*)) __end_vpci_array = .; #endif } :text @@ -182,7 +182,7 @@ SECTIONS #if defined(CONFIG_HAS_VPCI) && !defined(CONFIG_LATE_HWDOM) . = ALIGN(POINTER_ALIGN); __start_vpci_array = .; - *(.data.vpci) + *(SORT(.data.vpci.*)) __end_vpci_array = .; #endif } :text diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S index 7bd6fb51c3..70afedd31d 100644 --- a/xen/arch/x86/xen.lds.S +++ b/xen/arch/x86/xen.lds.S @@ -139,7 +139,7 @@ SECTIONS #if defined(CONFIG_HAS_VPCI) && defined(CONFIG_LATE_HWDOM) . = ALIGN(POINTER_ALIGN); __start_vpci_array = .; - *(.data.vpci) + *(SORT(.data.vpci.*)) __end_vpci_array = .; #endif } :text @@ -246,7 +246,7 @@ SECTIONS #if defined(CONFIG_HAS_VPCI) && !defined(CONFIG_LATE_HWDOM) . = ALIGN(POINTER_ALIGN); __start_vpci_array = .; - *(.data.vpci) + *(SORT(.data.vpci.*)) __end_vpci_array = .; #endif } :text diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c index 8d697b670b..234824c8b0 100644 --- a/xen/drivers/vpci/header.c +++ b/xen/drivers/vpci/header.c @@ -532,7 +532,7 @@ static int init_bars(struct pci_dev *pdev) return (cmd & PCI_COMMAND_MEMORY) ? modify_bars(pdev, true, false) : 0; } -REGISTER_VPCI_INIT(init_bars); +REGISTER_VPCI_INIT(init_bars, VPCI_PRIORITY_MIDDLE); /* * Local variables: diff --git a/xen/drivers/vpci/msi.c b/xen/drivers/vpci/msi.c index fb85d09e08..0e5e817cd6 100644 --- a/xen/drivers/vpci/msi.c +++ b/xen/drivers/vpci/msi.c @@ -267,7 +267,7 @@ static int init_msi(struct pci_dev *pdev) return 0; } -REGISTER_VPCI_INIT(init_msi); +REGISTER_VPCI_INIT(init_msi, VPCI_PRIORITY_LOW); void vpci_dump_msi(void) { diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h index c69598785c..a0e1706f51 100644 --- a/xen/include/xen/vpci.h +++ b/xen/include/xen/vpci.h @@ -15,9 +15,13 @@ typedef void vpci_write_t(const struct pci_dev *pdev, unsigned int reg, typedef int vpci_register_init_t(struct pci_dev *dev); -#define REGISTER_VPCI_INIT(x) \ +#define VPCI_PRIORITY_HIGH "1" +#define VPCI_PRIORITY_MIDDLE"5" +#define VPCI_PRIORITY_LOW "9" + +#define REGISTER_VPCI_INIT(x, p)\ static vpci_register_init_t *const x##_entry \ - __used_section(".data.vpci") = x + __used_section(".data.vpci." p) = x /* Add vPCI handlers to device. */ int __must_check vpci_add_handlers(struct pci_dev *dev); -- 2.16.2 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v9 01/11] vpci: introduce basic handlers to trap accesses to the PCI config space
This functionality is going to reside in vpci.c (and the corresponding vpci.h header), and should be arch-agnostic. The handlers introduced in this patch setup the basic functionality required in order to trap accesses to the PCI config space, and allow decoding the address and finding the corresponding handler that should handle the access (although no handlers are implemented). Note that the traps to the PCI IO ports registers (0xcf8/0xcfc) are setup inside of a x86 HVM file, since that's not shared with other arches. A new XEN_X86_EMU_VPCI x86 domain flag is added in order to signal Xen whether a domain should use the newly introduced vPCI handlers, this is only enabled for PVH Dom0 at the moment. A very simple user-space test is also provided, so that the basic functionality of the vPCI traps can be asserted. This has been proven quite helpful during development, since the logic to handle partial accesses or accesses that expand across multiple registers is not trivial. The handlers for the registers are added to a linked list that's keep sorted at all times. Both the read and write handlers support accesses that expand across multiple emulated registers and contain gaps not emulated. Signed-off-by: Roger Pau Monné [IO parts] Reviewed-by: Paul Durrant --- Cc: Andrew Cooper Cc: George Dunlap Cc: Ian Jackson Cc: Jan Beulich Cc: Konrad Rzeszutek Wilk Cc: Stefano Stabellini Cc: Tim Deegan Cc: Wei Liu Cc: Julien Grall Cc: Paul Durrant --- Changes since v9: - Introduce HAS_VPCI Kconfig option. - Drop Jan and Wei's RB (keep Paul's since the HAS_VPCI addition doesn't change IO code). Changes since v8: - Rebase on top of XSA-256. Changes since v7: - Constify d in vpci_portio_read. - ASSERT the correctness of the address in the read/write handlers. - Add newlines between non-fallthrough case statements. Changes since v6: - Align the vpci handlers in the linker script. - Switch add/remove register functions to take a vpci parameter instead of a pci_dev. - Expand comment of merge_result. - Return X86EMUL_UNHANDLEABLE if accessing cfc and cf8 is disabled. Changes since v5: - Use a spinlock per pci device. - Use the recently introduced pci_sbdf_t type. - Fix test harness to use the right handler type and the newly introduced lock. - Move the position of the vpci sections in the linker scripts. - Constify domain and pci_dev in vpci_{read/write}. - Fix typos in comments. - Use _XEN_VPCI_H_ as header guard. Changes since v4: * User-space test harness: - Do not redirect the output of the test. - Add main.c and emul.h as dependencies of the Makefile target. - Use the same rule to modify the vpci and list headers. - Remove underscores from local macro variables. - Add _check suffix to the test harness multiread function. - Change the value written by every different size in the multiwrite test. - Use { } to initialize the r16 and r20 arrays (instead of { 0 }). - Perform some of the read checks with the local variable directly. - Expand some comments. - Implement a dummy rwlock. * Hypervisor code: - Guard the linker script changes with CONFIG_HAS_PCI. - Rename vpci_access_check to vpci_access_allowed and make it return bool. - Make hvm_pci_decode_addr return the register as return value. - Use ~3 instead of 0xfffc to remove the register offset when checking accesses to IO ports. - s/head/prev in vpci_add_register. - Add parentheses around & in vpci_add_register. - Fix register removal. - Change the BUGs in vpci_{read/write}_hw helpers to ASSERT_UNREACHABLE. - Make merge_result static and change the computation of the mask to avoid using a uint64_t. - Modify vpci_read to only read from hardware the not-emulated gaps. - Remove the vpci_val union and use a uint32_t instead. - Change handler read type to return a uint32_t instead of modifying a variable passed by reference. - Constify the data opaque parameter of read handlers. - Change the size parameter of the vpci_{read/write} functions to unsigned int. - Place the array of initialization handlers in init.rodata or .rodata depending on whether late-hwdom is enabled. - Remove the pci_devs lock, assume the Dom0 is well behaved and won't remove the device while trying to access it. - Change the recursive spinlock into a rw lock for performance reasons. Changes since v3: * User-space test harness: - Fix spaces in container_of macro. - Implement a dummy locking functions. - Remove 'current' macro make current a pointer to the statically allocated vpcu. - Remove unneeded parentheses in the pci_conf_readX macros. - Fix the name of the write test macro. - Remove the dummy EXPORT_SYMBOL macro (this was needed by the RB code only). - Import the max macro. - Test all possible read/write size combinations with all possible emulated register sizes. - Introduce a test for register removal. * Hypervisor code: - Use a sorted list in order to store the config space ha
[Xen-devel] [PATCH v9 05/11] pci: add support to size ROM BARs to pci_size_mem_bar
Signed-off-by: Roger Pau Monné Reviewed-by: Jan Beulich --- Cc: Jan Beulich Cc: Andrew Cooper Cc: George Dunlap Cc: Ian Jackson Cc: Julien Grall Cc: Konrad Rzeszutek Wilk Cc: Stefano Stabellini Cc: Tim Deegan Cc: Wei Liu --- Changes since v6: - Remove the rom local variable. Changes since v5: - Use the flags field. - Introduce a mask local variable. - Simplify return. Changes since v4: - New in this version. --- xen/drivers/passthrough/pci.c | 28 ++-- xen/include/xen/pci.h | 1 + 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c index 190515b3c6..1751c66e34 100644 --- a/xen/drivers/passthrough/pci.c +++ b/xen/drivers/passthrough/pci.c @@ -610,11 +610,16 @@ unsigned int pci_size_mem_bar(pci_sbdf_t sbdf, unsigned int pos, uint32_t hi = 0, bar = pci_conf_read32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.func, pos); uint64_t size; - -ASSERT((bar & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_MEMORY); +bool is64bits = !(flags & PCI_BAR_ROM) && +(bar & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == PCI_BASE_ADDRESS_MEM_TYPE_64; +uint32_t mask = (flags & PCI_BAR_ROM) ? (uint32_t)PCI_ROM_ADDRESS_MASK + : (uint32_t)PCI_BASE_ADDRESS_MEM_MASK; + +ASSERT(!((flags & PCI_BAR_VF) && (flags & PCI_BAR_ROM))); +ASSERT((flags & PCI_BAR_ROM) || + (bar & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_MEMORY); pci_conf_write32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.func, pos, ~0); -if ( (bar & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == - PCI_BASE_ADDRESS_MEM_TYPE_64 ) +if ( is64bits ) { if ( flags & PCI_BAR_LAST ) { @@ -628,10 +633,9 @@ unsigned int pci_size_mem_bar(pci_sbdf_t sbdf, unsigned int pos, hi = pci_conf_read32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.func, pos + 4); pci_conf_write32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.func, pos + 4, ~0); } -size = pci_conf_read32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.func, pos) & - PCI_BASE_ADDRESS_MEM_MASK; -if ( (bar & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == - PCI_BASE_ADDRESS_MEM_TYPE_64 ) +size = pci_conf_read32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.func, + pos) & mask; +if ( is64bits ) { size |= (uint64_t)pci_conf_read32(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.func, pos + 4) << 32; @@ -643,14 +647,10 @@ unsigned int pci_size_mem_bar(pci_sbdf_t sbdf, unsigned int pos, size = -size; if ( paddr ) -*paddr = (bar & PCI_BASE_ADDRESS_MEM_MASK) | ((uint64_t)hi << 32); +*paddr = (bar & mask) | ((uint64_t)hi << 32); *psize = size; -if ( (bar & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == - PCI_BASE_ADDRESS_MEM_TYPE_64 ) -return 2; - -return 1; +return is64bits ? 2 : 1; } int pci_add_device(u16 seg, u8 bus, u8 devfn, diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h index 2f171a8dcc..4cfa774615 100644 --- a/xen/include/xen/pci.h +++ b/xen/include/xen/pci.h @@ -191,6 +191,7 @@ const char *parse_pci_seg(const char *, unsigned int *seg, unsigned int *bus, #define PCI_BAR_VF (1u << 0) #define PCI_BAR_LAST(1u << 1) +#define PCI_BAR_ROM (1u << 2) unsigned int pci_size_mem_bar(pci_sbdf_t sbdf, unsigned int pos, uint64_t *paddr, uint64_t *psize, unsigned int flags); -- 2.16.2 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v9 09/11] vpci/msi: add MSI handlers
Add handlers for the MSI control, address, data and mask fields in order to detect accesses to them and setup the interrupts as requested by the guest. Note that the pending register is not trapped, and the guest can freely read/write to it. Signed-off-by: Roger Pau Monné --- Cc: Jan Beulich Cc: Andrew Cooper Cc: George Dunlap Cc: Ian Jackson Cc: Julien Grall Cc: Konrad Rzeszutek Wilk Cc: Stefano Stabellini Cc: Tim Deegan Cc: Wei Liu Cc: Paul Durrant --- Changes since v8: - Add a FIXME about the lack of testing and a comment regarding the lack of cleaning done in the init_msi error path. - Free msi struct when cleaning up if an init function failed. - Remove the 'error' label of init_msi, the caller will already perform the cleaning. Changes since v7: - Don't store pci segment/bus on local variables. - Add an error label to init_msi. - Don't trap accesses to the PBA. - Fix msi_pending_bits_reg macro so it matches coding style. - Move the position of vectors in the vpci_msi struct. - Add a comment to clarify the expected state of vectors after pt_irq_create_bind and use XEN_DOMCTL_VMSI_X86_UNMASKED. Changes since v6: - Use domain_spin_lock_irq_desc instead of open coding it. - Reduce the size of printed debug messages. - Constify domain in vpci_dump_msi. - Lock domlist_read_lock before iterating over the list of domains. - Make max_vectors and vectors uint8_t. - Drop the vpci_ prefix from the static functions in msi.c. - Turn the booleans in vpci_msi into bitfields. - Apply the mask bits to all vectors when enabling msi. - Remove the pos field. - Remove the usage of __msi_set_{enable/disable}. - Update the bindings when the message or data fields are updated. - Make vpci_msi_arch_disable return void, it wasn't returning any error. - Prevent the guest from writing to the pending bits field, it's read only as defined in the spec. - Add the must_check attribute to vpci_msi_arch_enable. Changes since v5: - Update to new lock usage. - Change handlers to match the new type. - s/msi_flags/msi_gflags/, remove the local variables and use the new DOMCTL_VMSI_* defines. - Change the MSI arch function to take a vpci_msi instead of a vpci_arch_msi as parameter. - Fix the calculation of the guest vector for MSI injection to take into account the number of bits that can be modified. - Use INVALID_PIRQ everywhere. - Simplify exit path of vpci_msi_disable. - Remove the conditional when setting address64 and masking fields. - Add a process_pending_softirqs to the MSI dump loop. - Place the prototypes for the MSI arch-specific functions in xen/vpci.h. - Add parentheses around the INVALID_PIRQ definition. Changes since v4: - Fix commit message. - Change the ASSERTs in vpci_msi_arch_mask into ifs. - Introduce INVALID_PIRQ. - Destroy the partially created bindings in case of failure in vpci_msi_arch_enable. - Just take the pcidevs lock once in vpci_msi_arch_disable. - Print an error message in case of failure of pt_irq_destroy_bind. - Make vpci_msi_arch_init return void. - Constify the arch parameter of vpci_msi_arch_print. - Use fixed instead of cpu for msi redirection. - Separate the header includes in vpci/msi.c between xen and asm. - Store the number of configured vectors even if MSI is not enabled and always return it in vpci_msi_control_read. - Fix/add comments in vpci_msi_control_write to clarify intended behavior. - Simplify usage of masks in vpci_msi_address_{upper_}write. - Add comment to vpci_msi_mask_{read/write}. - Don't use MASK_EXTR in vpci_msi_mask_write. - s/msi_offset/pos/ in vpci_init_msi. - Move control variable setup closer to it's usage. - Use d%d in vpci_dump_msi. - Fix printing of bitfield mask in vpci_dump_msi. - Fix definition of MSI_ADDR_REDIRECTION_MASK. - Shuffle the layout of vpci_msi to minimize gaps. - Remove the error label in vpci_init_msi. Changes since v3: - Propagate changes from previous versions: drop xen_ prefix, drop return value from handlers, use the new vpci_val fields. - Use MASK_EXTR. - Remove the usage of GENMASK. - Add GFLAGS_SHIFT_DEST_ID and use it in msi_flags. - Add "arch" to the MSI arch specific functions. - Move the dumping of vPCI MSI information to dump_msi (key 'M'). - Remove the guest_vectors field. - Allow the guest to change the number of active vectors without having to disable and enable MSI. - Check the number of active vectors when parsing the disable mask. - Remove the debug messages from vpci_init_msi. - Move the arch-specific part of the dump handler to x86/hvm/vmsi.c. - Use trylock in the dump handler to get the vpci lock. Changes since v2: - Add an arch-specific abstraction layer. Note that this is only implemented for x86 currently. - Add a wrapper to detect MSI enabling for vPCI. --- NB: I've only been able to test this with devices using a single MSI interrupt and no mask register. I will try to find hardware that supports
[Xen-devel] [PATCH v9 11/11] vpci/msix: add MSI-X handlers
Add handlers for accesses to the MSI-X message control field on the PCI configuration space, and traps for accesses to the memory region that contains the MSI-X table and PBA. This traps detect attempts from the guest to configure MSI-X interrupts and properly sets them up. Note that accesses to the Table Offset, Table BIR, PBA Offset and PBA BIR are not trapped by Xen at the moment. Finally, turn the panic in the Dom0 PVH builder into a warning. Signed-off-by: Roger Pau Monné --- Cc: Jan Beulich Cc: Andrew Cooper Cc: George Dunlap Cc: Ian Jackson Cc: Julien Grall Cc: Konrad Rzeszutek Wilk Cc: Stefano Stabellini Cc: Tim Deegan Cc: Wei Liu Cc: Paul Durrant --- Changes since v8: - Call process_pending_softirqs between printing MSI-X entries. - Free msix struct in vpci_add_handlers. - Print only MSI or MSI-X if they are enabled. - Fix comment in update_entry. Changes since v7: - Switch vpci.h macros to inline functions. - Change vpci_msix_arch_print_entry into vpci_msix_arch_print and make it print all the entries. - Add a log message if rangeset_remove_range fails to remove the BAR MSI-related range. - Introduce a new update_entry to disable and enable a MSIX entry in order to either update or set it up. This removes open coding it in two different places. - Unify access checks in access_allowed. - Add newlines between switch cases. - Expand max_entries to 12 bits. Changes since v6: - Reduce the output of the debug keys. - Fix comments and code to match in vpci_msix_control_write. - Optimize size of the MSIX structure. - Convert 'tables[]' to a uint32_t in order to reduce the size of vpci_msix. Introduce some macros to make it easier to get the MSIX tables related data. - Limit size of the bool fields to 1 bit. - Remove the 'nr' field of vpci_msix_entry. The position can be calculated from the base of the entries array. - Drop the 'vpci_' prefix from the functions in msix.c, they are all static. - Remove the val local variable in control_read. - Initialize new_masked and new_enabled at declaration. - Recalculate the msix control value before writing it. - Remove the seg and bus local variables and use pdev->seg and pdev->bus instead. - Initialize msix at declaration in msix_{write/read}. - Add the must_check attribute to vpci_msix_arch_{enable/disable}_entry. Changes since v5: - Update lock usage. - Unbind/unmap PIRQs when MSIX is disabled. - Share the arch-specific MSIX code with the MSI functions. - Do not reference the MSIX memory areas from the PCI BARs fields, instead fetch the BIR and offset each time needed. - Add the '_entry' suffix to the MSIX arch functions. - Prefix the vMSIX macros with 'V'. - s/gdprintk/gprintk/ in msix.c - Make vpci_msix_access_check return bool, and change it's name to vpci_msix_access_allowed. - Join the first two ifs in vpci_msix_{read/write} into a single one. - Allow Dom0 to write to the PBA area. - Add a note that reads from the PBA area will need to be translated if the PBA it's not identity mapped. Changes since v4: - Remove parentheses around offsetof. - Add "being" to MSI-X enabling comment. - Use INVALID_PIRQ. - Add a simple sanity check to vpci_msix_arch_enable in order to detect wrong MSI-X entries more quickly. - Constify vpci_msix_arch_print entry argument. - s/cpu/fixed/ in vpci_msix_arch_print. - Dump the MSI-X info together with the MSI info. - Fix vpci_msix_control_write to take into account changes to the address and data fields when switching the function mask bit. - Only disable/enable the entries if the address or data fields have been updated. - Usew the BAR enable field to check if a BAR is mapped or not (instead of reading the command register for each device). - Fix error path in vpci_msix_read to set the return data to ~0. - Simplify mask usage in vpci_msix_write. - Cast data to uint64_t when shifting it 32 bits. - Fix writes to the table entry control register to take into account if the mask-all bit is set. - Add some comments to clarify the intended behavior of the code. - Align the PBA size to 64-bits. - Remove the error label in vpci_init_msix. - Try to compact the layout of the vpci_msix structure. - Remove the local table_bar and pba_bar variables from vpci_init_msix, they are used only once. Changes since v3: - Propagate changes from previous versions: remove xen_ prefix, use the new fields in vpci_val and remove the return value from handlers. - Remove the usage of GENMASK. - Mave the arch-specific parts of the dump routine to the x86/hvm/vmsi.c dump handler. - Chain the MSI-X dump handler to the 'M' debug key. - Fix the header BAR mappings so that the MSI-X regions inside of BARs are unmapped from the domain p2m in order for the handlers to work properly. - Unconditionally trap and forward accesses to the PBA MSI-X area. - Simplify the conditionals in vpci_msix_control_write. - Fix vpci_msi
[Xen-devel] [PATCH v9 07/11] vpci/bars: add handlers to map the BARs
Introduce a set of handlers that trap accesses to the PCI BARs and the command register, in order to snoop BAR sizing and BAR relocation. The command handler is used to detect changes to bit 2 (response to memory space accesses), and maps/unmaps the BARs of the device into the guest p2m. A rangeset is used in order to figure out which memory to map/unmap. This makes it easier to keep track of the possible overlaps with other BARs, and will also simplify MSI-X support, where certain regions of a BAR might be used for the MSI-X table or PBA. The BAR register handlers are used to detect attempts by the guest to size or relocate the BARs. Note that the long running BAR mapping and unmapping operations are deferred to be performed by hvm_io_pending, so that they can be safely preempted. Signed-off-by: Roger Pau Monné --- Cc: Ian Jackson Cc: Wei Liu Cc: Andrew Cooper Cc: George Dunlap Cc: Jan Beulich Cc: Julien Grall Cc: Konrad Rzeszutek Wilk Cc: Stefano Stabellini Cc: Tim Deegan Cc: Paul Durrant --- Changes since v8: - Do not pretend to support ARM in the map_range function. Explain the required changes in the comment. - Introduce PCI_HEADER_{NORMAL/BRIDGE}_NR_BARS defines. - Rename 'rom' boolean variable to 'rom_only', which is more descriptive of it's meaning. - Introduce vpci_remove_device which removes all handlers for a device. - Simplify error handling when modifying BARs mapping. Any error will cause the device to be unplugged (by calling vpci_remove_device). - Return an error code in modify_bars. Add comments describing why the error is sometimes ignored. Changes since v7: - Order includes. - Add newline between switch cases. - Fix typo in comment (hopping). - Wrap ternary conditional in parentheses. - Remove CONFIG_HAS_PCI gueard from sched.h vpci_vcpu usage. - Add comment regarding vpci_vcpu usage. - Move rom_enabled from BAR struct to header. - Do not protect vpci_vcpu with __XEN__ guards. Changes since v6: - s/vpci_check_pending/vpci_process_pending/. - Improve error handling in vpci_process_pending. - Add a comment that explains how vpci_check_bar_overlap works. - Add error messages to vpci_modify_bars and vpci_modify_rom. - Introduce vpci_hw_read16/32, in order to passthrough reads to the underlying hw. - Print BAR number on error in vpci_bar_write. - Place the CONFIG_HAS_PCI guards inside the vpci.h header and provide an empty vpci_vcpu structure for the !CONFIG_HAS_PCI case. - Define CONFIG_HAS_PCI in the test harness emul.h header before including vpci.h - Add ARM TODOs and an ARM-specific bodge to vpci_map_range due to the lack of preemption in {un}map_mmio_regions. - Make vpci_maybe_defer_map void. - Set rom_enabled in vpci_init_bars. - Defer enabling/disabling the memory decoding (or the ROM enable bit) until the memory has been mapped/unmapped. - Remove vpci_ prefix from static functions. - Use the same code in order to map the general BARs and the ROM BARs. - Remove the seg/bus local variables and use pdev->{seg,bus} instead. - Convert the bools in the BAR related structs into bool bitfields. - Add the must_check attribute to vpci_process_pending. - Open code check_bar_overlap inside modify_bars, which was it's only user. Changes since v5: - Switch to the new handler type. - Use pci_sbdf_t to size the BARs. - Use a single return for vpci_modify_bar. - Do not return an error code from vpci_modify_bars, just log the failure. - Remove the 'sizing' parameter. Instead just let the guest write directly to the BAR, and read the value back. This simplifies the BAR register handlers, specially the read one. - Ignore ROM BAR writes with memory decoding enabled and ROM enabled. - Do not propagate failures to setup the ROM BAR in vpci_init_bars. - Add preemption support to the BAR mapping/unmapping operations. Changes since v4: - Expand commit message to mention the reason behind the usage of rangesets. - Fix comment related to the inclusiveness of rangesets. - Fix off-by-one error in the calculation of the end of memory regions. - Store the state of the BAR (mapped/unmapped) in the vpci_bar enabled field, previously was only used by ROMs. - Fix double negation of return code. - Modify vpci_cmd_write so it has a single call to pci_conf_write16. - Print a warning when trying to write to the BAR with memory decoding enabled (and ignore the write). - Remove header_type local variable, it's used only once. - Move the read of the command register. - Restore previous command register value in the exit paths. - Only set address to INVALID_PADDR if the initial BAR value matches ~0 & PCI_BASE_ADDRESS_MEM_MASK. - Don't disable the enabled bit in the expansion ROM register, memory decoding is already disabled and takes precedence. - Don't use INVALID_PADDR, just set the initial BAR address to the value found in the hardware. - Introduce rom_enabled to store the status of the
[Xen-devel] [PATCH v9 03/11] x86/physdev: enable PHYSDEVOP_pci_mmcfg_reserved for PVH Dom0
So that MMCFG regions not present in the MCFG ACPI table can be added at run time by the hardware domain. Signed-off-by: Roger Pau Monné Reviewed-by: Jan Beulich Reviewed-by: Paul Durrant --- Cc: Jan Beulich Cc: Andrew Cooper Cc: Paul Durrant --- Changes since v7: - Add newline in hvm_physdev_op for non-fallthrough case. Changes since v6: - Do not return EEXIST if the same exact region is already tracked by Xen. Changes since v5: - Check for has_vpci before calling register_vpci_mmcfg_handler instead of checking for is_hvm_domain. Changes since v4: - Change the hardware_domain check in hvm_physdev_op to a vpci check. - Only register the MMCFG area, but don't scan it. Changes since v3: - New in this version. --- xen/arch/x86/hvm/hypercall.c | 5 + xen/arch/x86/hvm/io.c| 16 +++- xen/arch/x86/physdev.c | 11 +++ 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c index 5742dd1797..85eacd7d33 100644 --- a/xen/arch/x86/hvm/hypercall.c +++ b/xen/arch/x86/hvm/hypercall.c @@ -89,6 +89,11 @@ static long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) if ( !has_pirq(curr->domain) ) return -ENOSYS; break; + +case PHYSDEVOP_pci_mmcfg_reserved: +if ( !has_vpci(curr->domain) ) +return -ENOSYS; +break; } if ( !curr->hcall_compat ) diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c index 04425c064b..556810c126 100644 --- a/xen/arch/x86/hvm/io.c +++ b/xen/arch/x86/hvm/io.c @@ -507,10 +507,9 @@ static const struct hvm_mmio_ops vpci_mmcfg_ops = { .write = vpci_mmcfg_write, }; -int __hwdom_init register_vpci_mmcfg_handler(struct domain *d, paddr_t addr, - unsigned int start_bus, - unsigned int end_bus, - unsigned int seg) +int register_vpci_mmcfg_handler(struct domain *d, paddr_t addr, +unsigned int start_bus, unsigned int end_bus, +unsigned int seg) { struct hvm_mmcfg *mmcfg, *new = xmalloc(struct hvm_mmcfg); @@ -535,9 +534,16 @@ int __hwdom_init register_vpci_mmcfg_handler(struct domain *d, paddr_t addr, if ( new->addr < mmcfg->addr + mmcfg->size && mmcfg->addr < new->addr + new->size ) { +int ret = -EEXIST; + +if ( new->addr == mmcfg->addr && + new->start_bus == mmcfg->start_bus && + new->segment == mmcfg->segment && + new->size == mmcfg->size ) +ret = 0; write_unlock(&d->arch.hvm_domain.mmcfg_lock); xfree(new); -return -EEXIST; +return ret; } if ( list_empty(&d->arch.hvm_domain.mmcfg_regions) ) diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c index 380d36f6b9..984491c3dc 100644 --- a/xen/arch/x86/physdev.c +++ b/xen/arch/x86/physdev.c @@ -557,6 +557,17 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) ret = pci_mmcfg_reserved(info.address, info.segment, info.start_bus, info.end_bus, info.flags); +if ( !ret && has_vpci(currd) ) +{ +/* + * For HVM (PVH) domains try to add the newly found MMCFG to the + * domain. + */ +ret = register_vpci_mmcfg_handler(currd, info.address, + info.start_bus, info.end_bus, + info.segment); +} + break; } -- 2.16.2 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/pv: Fix guest crashes following f75b1a5247b "x86/pv: Drop int80_bounce from struct pv_vcpu"
On 14/03/18 13:58, Jan Beulich wrote: On 14.03.18 at 12:51, wrote: >> The original init_int80_direct_trap() was in fact buggy; `int $0x80` is not >> an >> exception. This went unnoticed for years because int80_bounce and >> trap_bounce >> were separate structures, but were combined by this change. >> >> Exception handling is different to interrupt handling for PV guests. By >> reusing trap_bounce, the following corner case can occur: >> >> * Handle a guest `int $0x80` instruction. Latches TBF_EXCEPTION into >>trap_bounce. >> * Handle an exception, which emulates to success (such as ptwr support), >>which leaves trap_bounce unmodified. >> * The exception exit path sees TBF_EXCEPTION set and re-injects the `int >>$0x80` a second time. > Oh, and then it was the clearing of trap_bounce after consuming it > in your conversion to C which masked the problem? Yes. > >> --- a/xen/arch/x86/x86_64/entry.S >> +++ b/xen/arch/x86/x86_64/entry.S >> @@ -373,10 +373,10 @@ UNLIKELY_END(msi_check) >> mov %cx, TRAPBOUNCE_cs(%rdx) >> mov %rdi, TRAPBOUNCE_eip(%rdx) >> >> -/* TB_flags = TBF_EXCEPTION | (TI_GET_IF(ti) ? TBF_INTERRUPT : 0); >> */ >> +/* TB_flags = (TI_GET_IF(ti) ? TBF_INTERRUPT : 0); */ >> testb $4, 0x80 * TRAPINFO_sizeof + TRAPINFO_flags(%rsi) >> setnz %cl >> -lea TBF_EXCEPTION(, %rcx, TBF_INTERRUPT), %ecx >> +lea (, %rcx, TBF_INTERRUPT), %ecx > With the immediate gone I think > > shl $3, %ecx > > would be more readable and perhaps no worse code wise (the > use of LEA was introduced in cases like this only to combine the > shift with the ORing in of other flags). I won't insist on that > change though (the more that there's no symbolic constant > available for that literal 3 right now), so with or without it > > Reviewed-by: Jan Beulich I'll do a followup patch. This particular pattern exists elsewhere, so might as well fix them all. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xenbaked.c: Avoid divide by zero issue on dump_stats()
On Wed, Mar 14, 2018 at 10:54:42AM +, George Dunlap wrote: > On Wed, Mar 14, 2018 at 2:24 AM, Konrad Rzeszutek Wilk > wrote: > > On Tue, Mar 13, 2018 at 06:38:24PM -0700, Joe Jin wrote: > >> run_time on dump_stats() maybe zero if break xenmon.py immediately after it > > > > s/maybe/can be/ > >> started, then xenbaked hit divide by zero fault. > > > > And: > > > > "Note that run_time is computed using two values which are retrieved using > > 'time' > > system call which gives us resolution in seconds." > > Is anyone still using this? Does it even work? I thought we had Yes as we found out as folks start coming out of the woods with the need to update their OS thanks to spectre. They use it with 'xenmon.py' combination. > talked about removing it at some point. > > -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/entry: Trivial nonfunctional fixes
>>> On 14.03.18 at 12:55, wrote: > * Drop unnecessary size suffixes > * The C pseudocode refers to a trap_info object, not trap_bounce. > > Signed-off-by: Andrew Cooper 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] x86/pv: Fix guest crashes following f75b1a5247b "x86/pv: Drop int80_bounce from struct pv_vcpu"
>>> On 14.03.18 at 12:51, wrote: > The original init_int80_direct_trap() was in fact buggy; `int $0x80` is not > an > exception. This went unnoticed for years because int80_bounce and > trap_bounce > were separate structures, but were combined by this change. > > Exception handling is different to interrupt handling for PV guests. By > reusing trap_bounce, the following corner case can occur: > > * Handle a guest `int $0x80` instruction. Latches TBF_EXCEPTION into >trap_bounce. > * Handle an exception, which emulates to success (such as ptwr support), >which leaves trap_bounce unmodified. > * The exception exit path sees TBF_EXCEPTION set and re-injects the `int >$0x80` a second time. Oh, and then it was the clearing of trap_bounce after consuming it in your conversion to C which masked the problem? > --- a/xen/arch/x86/x86_64/entry.S > +++ b/xen/arch/x86/x86_64/entry.S > @@ -373,10 +373,10 @@ UNLIKELY_END(msi_check) > mov %cx, TRAPBOUNCE_cs(%rdx) > mov %rdi, TRAPBOUNCE_eip(%rdx) > > -/* TB_flags = TBF_EXCEPTION | (TI_GET_IF(ti) ? TBF_INTERRUPT : 0); */ > +/* TB_flags = (TI_GET_IF(ti) ? TBF_INTERRUPT : 0); */ > testb $4, 0x80 * TRAPINFO_sizeof + TRAPINFO_flags(%rsi) > setnz %cl > -lea TBF_EXCEPTION(, %rcx, TBF_INTERRUPT), %ecx > +lea (, %rcx, TBF_INTERRUPT), %ecx With the immediate gone I think shl $3, %ecx would be more readable and perhaps no worse code wise (the use of LEA was introduced in cases like this only to combine the shift with the ORing in of other flags). I won't insist on that change though (the more that there's no symbolic constant available for that literal 3 right now), so with or without it 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] libxc/arm: initialise p2m_size to make gcc happy
>>> On 14.03.18 at 13:35, wrote: > On 14/03/18 12:32, Wei Liu wrote: >> Gcc with -O3 failed to spot the loop to initialise p2m_size runs at >> least once. > > It is -Og in this case, rather than -O3. My -O3 comment on the thread > was for the more generic cases. Yeah, it is likely the fact that gcc does _less_ optimization with -Og that makes it no longer spot that the variable can't be used uninitialized. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [xen-unstable-smoke bisection] complete build-arm64-xsm
On 3/14/18 5:54 AM, Wei Liu wrote: > On Wed, Mar 14, 2018 at 09:01:10AM +, Andrew Cooper wrote: >> On 14/03/2018 07:59, Jan Beulich wrote: >> On 14.03.18 at 03:29, wrote: branch xen-unstable-smoke xenbranch xen-unstable-smoke job build-arm64-xsm testid xen-build 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: b43501451733193b265de30fd79a764363a2a473 Bug not present: eef83fd2af0d4c78afec34c199c977fc97d8a0b3 Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/120707/ commit b43501451733193b265de30fd79a764363a2a473 Author: Doug Goldstein Date: Mon Mar 12 23:06:51 2018 -0500 tools: detect appropriate debug optimization level When building debug use -Og as the optimization level if its available, otherwise retain the use of -O0. -Og has been added by GCC to enable all optimizations that to not affect debugging while retaining full debugability. Signed-off-by: Doug Goldstein Acked-by: Wei Liu >>> Sadly altering optimization levels always has the potential of >>> triggering issues like this: >>> >>> xc_dom_arm.c: In function 'meminit': >>> xc_dom_arm.c:446:5: error: 'p2m_size' may be used uninitialized in this >>> function [-Werror=maybe-uninitialized] >>> for ( pfn = 0; pfn < p2m_size; pfn++ ) >>> ^ >>> cc1: all warnings being treated as errors >>> /home/osstest/build.120709.build-arm64-xsm/xen/tools/libxc/../../tools/Rules.mk:230: >>> recipe for target 'xc_dom_arm.o' failed >>> make[5]: *** [xc_dom_arm.o] Error 1 >> >> We really should be build testing things at all optimisation levels. We >> should be ashamed that -O3 gives build failures in most of our major >> components. (This is yet another item on my TODO list which I've not >> had time to complete.) > > I agree with your opinion in general. > > In this particular case, I think the compiler is to be blamed. p2m_size > should have been initialised in a previous loop by the time the code > comes to the place gcc complained. > > Wei. > Agreed. But the compiler used by osstest won't be changing any time soon to pick up fixes. What would the ARM maintainers like me to do? Initialize it to 0 at the top? -- Doug Goldstein 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 51/57] ARM: new VGIC: Add preliminary stub implementation
Hi, On 03/13/2018 03:55 PM, Andre Przywara wrote: Hi, On 09/03/18 18:18, Julien Grall wrote: Hi Andre, On 05/03/18 16:04, Andre Przywara wrote: The ARM arch code requires an interrupt controller emulation to implement vgic_clear_pending_irqs(), although it is suspected that it is actually not necessary. Go with a stub for now to make the linker happy. The implementation of that function is fundamentally wrong on the current vGIC for a few reasons: - lr_mask is reset but the LRs are not. This means when we context switch back, the LR might still be written and injecting unexpected interrupt (whoops). - both lists (inflight and pending) are cleared which means that a physical interrupt pending on that vCPU is lost forever (stay active in the physical so never going to fire again). Furthermore, I don't think that Xen business to reset the GIC on cpu_on. If anything should be done, then is it on CPU_off to migrate the current interrupts to another vCPU. But IIRC the OS is responsible for that. So I would kill that function. Any opinions? So I guess given that the patch is pretty small, we are good with keeping that for now, and solve this together with the old VGIC later. I am ok with that. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC PATCH 0/6] Using GitLab CI for build testing
On 3/14/18 4:45 AM, George Dunlap wrote: > On Tue, Mar 13, 2018 at 3:31 AM, Doug Goldstein wrote: >> Really early work on switching over to using GitLab CI over >> Travis CI. GitLab is a competitor to GitHub with some advantages >> such as an integrated CI system with a lot more flexibility >> and control. It additionally is fully open sourced unlike GitHub >> and Travis CI. We can even run an instance if that is preferred >> over using the hosted instance. >> >> This change uses GitLab CI's ability to use Docker based runners >> for running tests. With GitHub we also use a Docker based runner >> but we are limited to one Docker container that is then morphed >> a number of different ways. With this approach we can specify >> different Docker containers for every run (or use the same). By >> using different Docker containers we can build environments that >> match systems where Xen can and should build. Using this >> approach we should be able to cutdown on the number of surpise >> build failures encountered by users. >> >> An example run can be seen here: >> https://gitlab.com/cardoe/xen/pipelines/18789907 >> >> If there is interest in this I will move it over to the "xen-project" >> name space in the next version. >> >> Doug Goldstein (6): >> ci: add Dockerfile for CentOS 7.2 >> ci: add Dockerfile for Ubuntu 14.04 >> ci: add Dockerfile for Ubuntu 16.04 >> ci: add Dockerfile for Debian jessie >> ci: add cfg to use GitLab CI to build >> ci: add a README about the containers >> >> .gitlab-ci.yml | 34 ++- >> extras/testing/README.md| 29 ++- >> extras/testing/centos/CentOS-7.2.repo | 35 ++- >> extras/testing/centos/Dockerfile.7.2| 41 ++- >> extras/testing/debian/Dockerfile.jessie | 21 +- >> extras/testing/ubuntu/Dockerfile.trusty | 21 +- >> extras/testing/ubuntu/Dockerfile.xenial | 21 +- > > "extras" is a bit generic. What about something like "automation/build"? > > (You knew this bike shed wasn't going to get in without *some* > discussion of the color!) > > -George > Ha. Thank you. So in the same thread I'll move the helper script that the CI will use into "automation/scripts"? Currently we have "scripts/travis-build" but there are going to be some more (specifically for ARM) -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel