Re: [Xen-devel] [PATCH for-4.11] x86/SVM: Fix intercepted {RD, WR}MSR for the SYS{CALL, ENTER} MSRs
>>> On 24.04.18 at 20:51, wrote:
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1883,6 +1883,22 @@ static int svm_msr_read_intercept(unsigned int msr,
> uint64_t *msr_content)
> switch ( msr )
> {
> case MSR_IA32_SYSENTER_CS:
> +case MSR_IA32_SYSENTER_ESP:
> +case MSR_IA32_SYSENTER_EIP:
These three do not require sync-ing, as their values aren't read from the VMCB.
(They do require sync-ing on the write path).
I also don't think this is going to fully resolve Razvan's issue (not the least
because the code paths you adjust aren't involved in his scenario): As
pointed out in a private mail, I think vmcb_in_sync needs to start out as
true for a vCPU, and may need setting to true upon context set and/or
reset/init emulation.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/1] drm/xen-zcopy: Add Xen zero-copy helper DRM driver
On Wed, Apr 25, 2018 at 09:07:07AM +0300, Oleksandr Andrushchenko wrote: > On 04/24/2018 11:35 PM, Dongwon Kim wrote: > > Had a meeting with Daniel and talked about bringing out generic > > part of hyper-dmabuf to the userspace, which means we most likely > > reuse IOCTLs defined in xen-zcopy for our use-case if we follow > > his suggestion. > I will still have kernel side API, so backends/frontends implemented > in the kernel can access that functionality as well. > > > > So assuming we use these IOCTLs as they are, > > Several things I would like you to double-check.. > > > > 1. returning gref as is to the user space is still unsafe because > > it is a constant, easy to guess and any process that hijacks it can easily > > exploit the buffer. So I am wondering if it's possible to keep dmabuf-to > > -gref or gref-to-dmabuf in kernel space and add other layers on top > > of those in actual IOCTLs to add some safety.. We introduced flink like > > hyper_dmabuf_id including random number but many says even that is still > > not safe. > Yes, it is generally unsafe. But even if we have implemented > the approach you have in hyper-dmabuf or similar, what stops > malicious software from doing the same with the existing gntdev UAPI? > No need to brute force new UAPI if there is a simpler one. > That being said, I'll put security aside at the first stage, > but of course we can start investigating ways to improve > (I assume you already have use-cases where security issues must > be considered, so, probably you can tell more on what was investigated > so far). Maybe a bit more context here: So in graphics we have this old flink approach for buffer sharing with processes, and it's unsafe because way too easy to guess the buffer handles. And anyone with access to the graphics driver can then import that buffer object. We switched to file descriptor passing to make sure only the intended recipient can import a buffer. So at the vm->vm level it sounds like grefs are safe, because they're only for a specific other guest (or sets of guests, not sure about). That means security is only within the OS. For that you need to make sure that unpriviledge userspace simply can't ever access a gref. If that doesn't work out, then I guess we should improve the xen gref stuff to have a more secure cookie. > > 2. maybe we could take hypervisor-independent process (e.g. SGT<->page) > > out of xen-zcopy and put those in a new helper library. > I believe this can be done, but at the first stage I would go without > that helper library, so it is clearly seen what can be moved to it later > (I know that you want to run ACRN as well, but can I run it on ARM? ;) There's already helpers for walking sgtables and adding pages/enumerating pages. I don't think we need more. > > 3. please consider the case where original DMA-BUF's first offset > > and last length are not 0 and PAGE_SIZE respectively. I assume current > > xen-zcopy only supports page-aligned buffer with PAGE_SIZE x n big. > Hm, what is the use-case for that? dma-buf is always page-aligned. That's a hard constraint of the linux dma-buf interface spec. -Daniel > > thanks, > > DW > Thank you, > Oleksandr > > On Tue, Apr 24, 2018 at 02:59:39PM +0300, Oleksandr Andrushchenko wrote: > > > On 04/24/2018 02:54 PM, Daniel Vetter wrote: > > > > On Mon, Apr 23, 2018 at 03:10:35PM +0300, Oleksandr Andrushchenko wrote: > > > > > On 04/23/2018 02:52 PM, Wei Liu wrote: > > > > > > On Fri, Apr 20, 2018 at 02:25:20PM +0300, Oleksandr Andrushchenko > > > > > > wrote: > > > > > > > > > the gntdev. > > > > > > > > > > > > > > > > > > I think this is generic enough that it could be implemented > > > > > > > > > by a > > > > > > > > > device not tied to Xen. AFAICT the hyper_dma guys also wanted > > > > > > > > > something similar to this. > > > > > > > > You can't just wrap random userspace memory into a dma-buf. > > > > > > > > We've just had > > > > > > > > this discussion with kvm/qemu folks, who proposed just that, > > > > > > > > and after a > > > > > > > > bit of discussion they'll now try to have a driver which just > > > > > > > > wraps a > > > > > > > > memfd into a dma-buf. > > > > > > > So, we have to decide either we introduce a new driver > > > > > > > (say, under drivers/xen/xen-dma-buf) or extend the existing > > > > > > > gntdev/balloon to support dma-buf use-cases. > > > > > > > > > > > > > > Can anybody from Xen community express their preference here? > > > > > > > > > > > > > Oleksandr talked to me on IRC about this, he said a few IOCTLs need > > > > > > to > > > > > > be added to either existing drivers or a new driver. > > > > > > > > > > > > I went through this thread twice and skimmed through the relevant > > > > > > documents, but I couldn't see any obvious pros and cons for either > > > > > > approach. So I don't really have an opinion on this. > > > > > > > > > > > > But, assuming if implemented in existing drivers, those IOCTLs need > > > > > > to
[Xen-devel] [libvirt test] 122383: trouble: broken/pass
flight 122383 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/122383/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-libvirt-raw broken test-armhf-armhf-libvirt-raw 4 host-install(4)broken REGR. vs. 122344 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail like 122344 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 122344 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-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 12 migrate-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 13 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt 14 saverestore-support-checkfail never pass version targeted for testing: libvirt 6009d5124f1045b38bc499b8a171e5fd7f304a8a baseline version: libvirt 4ac43975d514fca900896ddb3e54ef9f145920fe Last test of basis 122344 2018-04-17 04:20:20 Z8 days Testing same since 122383 2018-04-24 04:19:50 Z1 days1 attempts People who touched revisions under test: Andrea Bolognani Clementine Hayat Daniel P. Berrangé Erik Skultety Jim Fehlig Jiri Denemark John Ferlan Ján Tomko Marek Marczykowski-Górecki Martin Kletzander Michal Privoznik Nikolay Shirokovskiy Peter Krempa Pino Toscano Rainer Müller Richard W.M. Jones Sukrit Bhatnagar Viktor Mihajlovski jobs: build-amd64-xsm pass build-arm64-xsm pass build-armhf-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass test-amd64-amd64-libvirt-xsm pass test-arm64-arm64-libvirt-xsm pass test-armhf-armhf-libvirt-xsm pass test-amd64-i386-libvirt-xsm pass test-amd64-amd64-libvirt pass test-arm64-arm64-libvirt pass test-armhf-armhf-libvirt pass test-amd64-i386-libvirt pass test-amd64-amd64-libvirt-pairpass test-amd64-i386-libvirt-pair pass test-arm64-arm64-libvirt-qcow2 pass test-armhf-armhf-libvirt-raw broken test-amd64-amd64-libvirt-vhd 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.xenproj
Re: [Xen-devel] [PATCH 0/1] drm/xen-zcopy: Add Xen zero-copy helper DRM driver
On 24/04/18 22:35, Dongwon Kim wrote: > Had a meeting with Daniel and talked about bringing out generic > part of hyper-dmabuf to the userspace, which means we most likely > reuse IOCTLs defined in xen-zcopy for our use-case if we follow > his suggestion. > > So assuming we use these IOCTLs as they are, > Several things I would like you to double-check.. > > 1. returning gref as is to the user space is still unsafe because > it is a constant, easy to guess and any process that hijacks it can easily > exploit the buffer. So I am wondering if it's possible to keep dmabuf-to > -gref or gref-to-dmabuf in kernel space and add other layers on top > of those in actual IOCTLs to add some safety.. We introduced flink like > hyper_dmabuf_id including random number but many says even that is still > not safe. grefs are usable by root only. When you have root access in dom0 you can do evil things to all VMs even without using grants. That is in no way different to root being able to control all other processes on the system. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/1] drm/xen-zcopy: Add Xen zero-copy helper DRM driver
On 04/24/2018 11:35 PM, Dongwon Kim wrote: Had a meeting with Daniel and talked about bringing out generic part of hyper-dmabuf to the userspace, which means we most likely reuse IOCTLs defined in xen-zcopy for our use-case if we follow his suggestion. I will still have kernel side API, so backends/frontends implemented in the kernel can access that functionality as well. So assuming we use these IOCTLs as they are, Several things I would like you to double-check.. 1. returning gref as is to the user space is still unsafe because it is a constant, easy to guess and any process that hijacks it can easily exploit the buffer. So I am wondering if it's possible to keep dmabuf-to -gref or gref-to-dmabuf in kernel space and add other layers on top of those in actual IOCTLs to add some safety.. We introduced flink like hyper_dmabuf_id including random number but many says even that is still not safe. Yes, it is generally unsafe. But even if we have implemented the approach you have in hyper-dmabuf or similar, what stops malicious software from doing the same with the existing gntdev UAPI? No need to brute force new UAPI if there is a simpler one. That being said, I'll put security aside at the first stage, but of course we can start investigating ways to improve (I assume you already have use-cases where security issues must be considered, so, probably you can tell more on what was investigated so far). 2. maybe we could take hypervisor-independent process (e.g. SGT<->page) out of xen-zcopy and put those in a new helper library. I believe this can be done, but at the first stage I would go without that helper library, so it is clearly seen what can be moved to it later (I know that you want to run ACRN as well, but can I run it on ARM? ;) 3. please consider the case where original DMA-BUF's first offset and last length are not 0 and PAGE_SIZE respectively. I assume current xen-zcopy only supports page-aligned buffer with PAGE_SIZE x n big. Hm, what is the use-case for that? thanks, DW Thank you, Oleksandr On Tue, Apr 24, 2018 at 02:59:39PM +0300, Oleksandr Andrushchenko wrote: On 04/24/2018 02:54 PM, Daniel Vetter wrote: On Mon, Apr 23, 2018 at 03:10:35PM +0300, Oleksandr Andrushchenko wrote: On 04/23/2018 02:52 PM, Wei Liu wrote: On Fri, Apr 20, 2018 at 02:25:20PM +0300, Oleksandr Andrushchenko wrote: the gntdev. I think this is generic enough that it could be implemented by a device not tied to Xen. AFAICT the hyper_dma guys also wanted something similar to this. You can't just wrap random userspace memory into a dma-buf. We've just had this discussion with kvm/qemu folks, who proposed just that, and after a bit of discussion they'll now try to have a driver which just wraps a memfd into a dma-buf. So, we have to decide either we introduce a new driver (say, under drivers/xen/xen-dma-buf) or extend the existing gntdev/balloon to support dma-buf use-cases. Can anybody from Xen community express their preference here? Oleksandr talked to me on IRC about this, he said a few IOCTLs need to be added to either existing drivers or a new driver. I went through this thread twice and skimmed through the relevant documents, but I couldn't see any obvious pros and cons for either approach. So I don't really have an opinion on this. But, assuming if implemented in existing drivers, those IOCTLs need to be added to different drivers, which means userspace program needs to write more code and get more handles, it would be slightly better to implement a new driver from that perspective. If gntdev/balloon extension is still considered: All the IOCTLs will be in gntdev driver (in current xen-zcopy terminology): I was lazy to change dumb to dma-buf, so put this notice ;) - DRM_ICOTL_XEN_ZCOPY_DUMB_FROM_REFS - DRM_IOCTL_XEN_ZCOPY_DUMB_TO_REFS - DRM_IOCTL_XEN_ZCOPY_DUMB_WAIT_FREE s/DUMB/DMA_BUF/ please. This is generic dma-buf, it has nothing to do with the dumb scanout buffer support in the drm/gfx subsystem. This here can be used for any zcopy sharing among guests (as long as your endpoints understands dma-buf, which most relevant drivers do). Of course, please see above -Daniel Balloon driver extension, which is needed for contiguous/DMA buffers, will be to provide new *kernel API*, no UAPI is needed. Wei. Thank you, Oleksandr ___ dri-devel mailing list dri-de...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.11] x86/SVM: Fix intercepted {RD, WR}MSR for the SYS{CALL, ENTER} MSRs
On 24/04/18 20:51, Andrew Cooper wrote:
> By default, the SYSCALL MSRs are not intercepted, and accesses are completed
> by hardware. The SYSENTER MSRs are intercepted for cross-vendor
> purposes (albeit needlessly in the common case), and are fully emulated.
>
> However, {RD,WR}MSR instructions which happen to be emulated (FEP,
> introspection, or older versions of Xen which intercepted #UD), or when the
> MSRs are explicitly intercepted (introspection), will be completed
> incorrectly.
>
> svm_msr_read_intercept() appears to return the correct values, but only
> because of the default read-everything case (which is going to disappear), and
> that in vcpu context, hardware should have the guest values in context.
> Update the read path to explicitly sync the VMCB and complete the accesses,
> rather than falling all the way through to the default case.
>
> svm_msr_write_intercept() silently discard all updates. Synchronise the VMCB
> for all applicable MSRs, and implement suitable checks. The actual behaviour
> of AMD hardware is to truncate the SYSENTER and SFMASK MSRs at 32 bits, but
> this isn't implemented yet to remain compatible with the cross-vendor case.
>
> Drop one bit of trailing whitespace while modifing this area of the code.
>
> Signed-off-by: Andrew Cooper
> ---
> CC: Jan Beulich
> CC: Juergen Gross
> CC: Boris Ostrovsky
> CC: Suravee Suthikulpanit
> CC: Brian Woods
>
> Juergen: As this patch probably wants backporting to the stable branches, it
> probably wants to go into 4.11 at this point.
Release-acked-by: Juergen Gross
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH 09/16] os-posix: cleanup: Replace fprintfs with error_report in change_process_uid
On 24.04.2018 19:58, Ian Jackson wrote:
> I'm going to be editing this function and it makes sense to clean up
> this style problem in advance.
>
> Signed-off-by: Ian Jackson
> CC: Paolo Bonzini
> CC: Markus Armbruster
> CC: Daniel P. Berrange
> CC: Michael Tokarev
> ---
> os-posix.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/os-posix.c b/os-posix.c
> index b9c2343..560db95 100644
> --- a/os-posix.c
> +++ b/os-posix.c
> @@ -167,20 +167,20 @@ static void change_process_uid(void)
> {
> if (user_pwd) {
> if (setgid(user_pwd->pw_gid) < 0) {
> -fprintf(stderr, "Failed to setgid(%d)\n", user_pwd->pw_gid);
> +error_report("Failed to setgid(%d)", user_pwd->pw_gid);
> exit(1);
> }
> if (initgroups(user_pwd->pw_name, user_pwd->pw_gid) < 0) {
> -fprintf(stderr, "Failed to initgroups(\"%s\", %d)\n",
> -user_pwd->pw_name, user_pwd->pw_gid);
> +error_report("Failed to initgroups(\"%s\", %d)",
> + user_pwd->pw_name, user_pwd->pw_gid);
> exit(1);
> }
> if (setuid(user_pwd->pw_uid) < 0) {
> -fprintf(stderr, "Failed to setuid(%d)\n", user_pwd->pw_uid);
> +error_report("Failed to setuid(%d)", user_pwd->pw_uid);
> exit(1);
> }
> if (setuid(0) != -1) {
> -fprintf(stderr, "Dropping privileges failed\n");
> +error_report("Dropping privileges failed");
> exit(1);
> }
> }
>
Reviewed-by: Thomas Huth
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH 14/16] os-posix: cleanup: Replace fprintf with error_report in remaining call sites
On 24.04.2018 19:58, Ian Jackson wrote:
> Signed-off-by: Ian Jackson
> CC: Paolo Bonzini
> CC: Markus Armbruster
> CC: Daniel P. Berrange
> CC: Michael Tokarev
> Reviewed-by: Philippe Mathieu-Daudé
> ---
> v8: Remove one remaining spurious "\n"
> v7: New patch
> ---
> os-posix.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/os-posix.c b/os-posix.c
> index 0f59566..a2ba50d 100644
> --- a/os-posix.c
> +++ b/os-posix.c
> @@ -129,7 +129,7 @@ void os_set_proc_name(const char *s)
> exit(1);
> }
> #else
> -fprintf(stderr, "Change of process name not supported by your OS\n");
> +error_report("Change of process name not supported by your OS");
> exit(1);
> #endif
> }
> @@ -243,7 +243,7 @@ static void change_root(void)
> {
> if (chroot_dir) {
> if (chroot(chroot_dir) < 0) {
> -fprintf(stderr, "chroot failed\n");
> +error_report("chroot failed");
> exit(1);
> }
> if (chdir("/")) {
>
Maybe merge this with patch 9/16?
Anyway:
Reviewed-by: Thomas Huth
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 16/16] configure: do_compiler: Dump some extra info under bash
On 24.04.2018 19:58, Ian Jackson wrote:
> This makes it much easier to find a particular thing in config.log.
>
> We have to use the ${BASH_LINENO[*]} syntax which is a syntax error in
> other shells, so test what shell we are running and use eval.
>
> The extra output is only printed if configure is run with bash. On
> systems where /bin/sh is not bash, it is necessary to say bash
> ./configure to get the extra debug info in the log.
>
> Suggested-by: Eric Blake
> Signed-off-by: Ian Jackson
> CC: Kent R. Spillner
> CC: Janosch Frank
> CC: Thomas Huth
> CC: Peter Maydell
> CC: Paolo Bonzini
> ---
> v8: Fix so that it actually works as intended with bash.
> v6: Fix commit message wording.
> v4: No longer tag this patch RFC.
> ---
> configure | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/configure b/configure
> index aa35aef..f9ba9ea 100755
> --- a/configure
> +++ b/configure
> @@ -60,6 +60,11 @@ do_compiler() {
> # is compiler binary to execute.
> local compiler="$1"
> shift
> +if test -n "$BASH_VERSION"; then eval '
> +echo >>config.log "
> +funcs: ${FUNCNAME[*]}
> +lines: ${BASH_LINENO[*]}"
> +'; fi
> echo $compiler "$@" >> config.log
> $compiler "$@" >> config.log 2>&1 || return $?
> # Test passed. If this is an --enable-werror build, rerun
I just applied the patch and had a look at config.log, and this looks
useful indeed.
Tested-by: Thomas Huth
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable test] 122363: regressions - FAIL
flight 122363 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/122363/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemuu-ovmf-amd64 16 guest-localmigrate/x10 fail REGR. vs. 122343 test-amd64-amd64-xl-qemuu-win7-amd64 7 xen-boot fail REGR. vs. 122343 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 122332 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail like 122343 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 122343 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 122343 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 122343 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 122343 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 122343 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 122343 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 122343 test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 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-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-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-armhf-xl-rtds 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-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-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-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass version targeted for testing: xen 25b0dad541e31bd892d57cbeafe8e0c0bf4e8385 baseline version: xen a6aa678fa380e9369cc44701a181142322b3a4b0 Last test of basis 122343 2018-04-17 04:06:26 Z7 days Testing same since 122363 2018-04-23 12:06:44 Z1 days1 attempts People who touched
Re: [Xen-devel] [PATCH for-4.11] x86/SVM: Fix intercepted {RD, WR}MSR for the SYS{CALL, ENTER} MSRs
On 04/24/2018 02:51 PM, Andrew Cooper wrote:
> By default, the SYSCALL MSRs are not intercepted, and accesses are completed
> by hardware. The SYSENTER MSRs are intercepted for cross-vendor
> purposes (albeit needlessly in the common case), and are fully emulated.
>
> However, {RD,WR}MSR instructions which happen to be emulated (FEP,
> introspection, or older versions of Xen which intercepted #UD), or when the
> MSRs are explicitly intercepted (introspection), will be completed
> incorrectly.
>
> svm_msr_read_intercept() appears to return the correct values, but only
> because of the default read-everything case (which is going to disappear), and
> that in vcpu context, hardware should have the guest values in context.
> Update the read path to explicitly sync the VMCB and complete the accesses,
> rather than falling all the way through to the default case.
>
> svm_msr_write_intercept() silently discard all updates. Synchronise the VMCB
> for all applicable MSRs, and implement suitable checks. The actual behaviour
> of AMD hardware is to truncate the SYSENTER and SFMASK MSRs at 32 bits, but
> this isn't implemented yet to remain compatible with the cross-vendor case.
>
> Drop one bit of trailing whitespace while modifing this area of the code.
>
> Signed-off-by: Andrew Cooper
> ---
> CC: Jan Beulich
> CC: Juergen Gross
> CC: Boris Ostrovsky
> CC: Suravee Suthikulpanit
> CC: Brian Woods
Reviewed-by: Boris Ostrovsky
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 15/16] os-posix: cleanup: Replace perror with error_report
On Tue, Apr 24, 2018 at 10:58 AM, Ian Jackson wrote:
> perror() is defined to fprintf(stderr,...). HACKING says
> fprintf(stderr,...) is wrong. So perror() is too.
>
> Signed-off-by: Ian Jackson
> CC: Paolo Bonzini
> CC: Markus Armbruster
> CC: Daniel P. Berrange
> CC: Michael Tokarev
> CC: Alistair Francis
> Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Alistair Francis
Alistair
> ---
> v7: New patch
> ---
> os-posix.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/os-posix.c b/os-posix.c
> index a2ba50d..24eb700 100644
> --- a/os-posix.c
> +++ b/os-posix.c
> @@ -125,7 +125,7 @@ void os_set_proc_name(const char *s)
> /* Could rewrite argv[0] too, but that's a bit more complicated.
> This simple way is enough for `top'. */
> if (prctl(PR_SET_NAME, name)) {
> -perror("unable to change process name");
> +error_report("unable to change process name: %s", strerror(errno));
> exit(1);
> }
> #else
> @@ -247,7 +247,7 @@ static void change_root(void)
> exit(1);
> }
> if (chdir("/")) {
> -perror("not able to chdir to /");
> +error_report("not able to chdir to /: %s", strerror(errno));
> exit(1);
> }
> }
> @@ -309,7 +309,7 @@ void os_setup_post(void)
>
> if (daemonize) {
> if (chdir("/")) {
> -perror("not able to chdir to /");
> +error_report("not able to chdir to /: %s", strerror(errno));
> exit(1);
> }
> TFR(fd = qemu_open("/dev/null", O_RDWR));
> @@ -383,7 +383,7 @@ int os_mlock(void)
>
> ret = mlockall(MCL_CURRENT | MCL_FUTURE);
> if (ret < 0) {
> -perror("mlockall");
> +error_report("mlockall: %s", strerror(errno));
> }
>
> return ret;
> --
> 2.1.4
>
>
> ___
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/1] drm/xen-zcopy: Add Xen zero-copy helper DRM driver
Had a meeting with Daniel and talked about bringing out generic part of hyper-dmabuf to the userspace, which means we most likely reuse IOCTLs defined in xen-zcopy for our use-case if we follow his suggestion. So assuming we use these IOCTLs as they are, Several things I would like you to double-check.. 1. returning gref as is to the user space is still unsafe because it is a constant, easy to guess and any process that hijacks it can easily exploit the buffer. So I am wondering if it's possible to keep dmabuf-to -gref or gref-to-dmabuf in kernel space and add other layers on top of those in actual IOCTLs to add some safety.. We introduced flink like hyper_dmabuf_id including random number but many says even that is still not safe. 2. maybe we could take hypervisor-independent process (e.g. SGT<->page) out of xen-zcopy and put those in a new helper library. 3. please consider the case where original DMA-BUF's first offset and last length are not 0 and PAGE_SIZE respectively. I assume current xen-zcopy only supports page-aligned buffer with PAGE_SIZE x n big. thanks, DW On Tue, Apr 24, 2018 at 02:59:39PM +0300, Oleksandr Andrushchenko wrote: > On 04/24/2018 02:54 PM, Daniel Vetter wrote: > >On Mon, Apr 23, 2018 at 03:10:35PM +0300, Oleksandr Andrushchenko wrote: > >>On 04/23/2018 02:52 PM, Wei Liu wrote: > >>>On Fri, Apr 20, 2018 at 02:25:20PM +0300, Oleksandr Andrushchenko wrote: > >> the gntdev. > >> > >>I think this is generic enough that it could be implemented by a > >>device not tied to Xen. AFAICT the hyper_dma guys also wanted > >>something similar to this. > >You can't just wrap random userspace memory into a dma-buf. We've just > >had > >this discussion with kvm/qemu folks, who proposed just that, and after a > >bit of discussion they'll now try to have a driver which just wraps a > >memfd into a dma-buf. > So, we have to decide either we introduce a new driver > (say, under drivers/xen/xen-dma-buf) or extend the existing > gntdev/balloon to support dma-buf use-cases. > > Can anybody from Xen community express their preference here? > > >>>Oleksandr talked to me on IRC about this, he said a few IOCTLs need to > >>>be added to either existing drivers or a new driver. > >>> > >>>I went through this thread twice and skimmed through the relevant > >>>documents, but I couldn't see any obvious pros and cons for either > >>>approach. So I don't really have an opinion on this. > >>> > >>>But, assuming if implemented in existing drivers, those IOCTLs need to > >>>be added to different drivers, which means userspace program needs to > >>>write more code and get more handles, it would be slightly better to > >>>implement a new driver from that perspective. > >>If gntdev/balloon extension is still considered: > >> > >>All the IOCTLs will be in gntdev driver (in current xen-zcopy terminology): > I was lazy to change dumb to dma-buf, so put this notice ;) > >> - DRM_ICOTL_XEN_ZCOPY_DUMB_FROM_REFS > >> - DRM_IOCTL_XEN_ZCOPY_DUMB_TO_REFS > >> - DRM_IOCTL_XEN_ZCOPY_DUMB_WAIT_FREE > >s/DUMB/DMA_BUF/ please. This is generic dma-buf, it has nothing to do with > >the dumb scanout buffer support in the drm/gfx subsystem. This here can be > >used for any zcopy sharing among guests (as long as your endpoints > >understands dma-buf, which most relevant drivers do). > Of course, please see above > >-Daniel > > > >>Balloon driver extension, which is needed for contiguous/DMA > >>buffers, will be to provide new *kernel API*, no UAPI is needed. > >> > >>>Wei. > >>Thank you, > >>Oleksandr > >>___ > >>dri-devel mailing list > >>dri-de...@lists.freedesktop.org > >>https://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [ovmf baseline-only test] 74639: all pass
This run is configured for baseline tests only. flight 74639 ovmf real [real] http://osstest.xs.citrite.net/~osstest/testlogs/logs/74639/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf ee4dc24f57c32a445e7c747396c9bfbd8b221568 baseline version: ovmf 55f67014d7b4a1228754313917ccca5539764802 Last test of basis74631 2018-04-17 14:52:22 Z7 days Testing same since74639 2018-04-24 18:22:13 Z0 days1 attempts People who touched revisions under test: Ard Biesheuvel Carsey, Jaben Carsey, Jaben Dandan Bi Evan Lloyd Hao Wu Jaben Carsey Laszlo Ersek Liming Gao Ruiyu Ni Sami Mujawar Star Zeng Yunhua Feng jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.xs.citrite.net logs: /home/osstest/logs images: /home/osstest/images Logs, config files, etc. are available at http://osstest.xs.citrite.net/~osstest/testlogs/logs Test harness code can be found at http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary Push not applicable. (No revision log; it would be 615 lines long.) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-4.9 test] 122360: regressions - trouble: broken/fail/pass
flight 122360 linux-4.9 real [real] http://logs.test-lab.xenproject.org/osstest/logs/122360/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemuu-win10-i386 broken test-amd64-amd64-xl-qemuu-win10-i386 4 host-install(4) broken REGR. vs. 122289 test-armhf-armhf-examine11 examine-serial/bootloader fail REGR. vs. 122289 test-armhf-armhf-examine 12 examine-serial/kernelfail REGR. vs. 122289 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 16 guest-localmigrate/x10 fail like 122272 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 122289 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 122289 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 122289 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 122289 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 122289 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-xl-pvshim12 guest-start fail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-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-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail 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-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-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-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop 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-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass version targeted for testing: linuxeedaf21fb32353c81ea5eb7c910a1acd958523d1 baseline version: linuxcc0eb4dd504b8a0adab865a9488297
[Xen-devel] [qemu-mainline baseline-only test] 74638: tolerable FAIL
This run is configured for baseline tests only. flight 74638 qemu-mainline real [real] http://osstest.xs.citrite.net/~osstest/testlogs/logs/74638/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win10-i386 10 windows-install fail baseline untested test-armhf-armhf-libvirt-xsm 12 guest-start fail baseline untested test-armhf-armhf-xl-xsm 12 guest-start fail baseline untested test-armhf-armhf-libvirt 12 guest-start fail baseline untested test-armhf-armhf-xl-credit2 12 guest-start fail baseline untested test-armhf-armhf-xl-multivcpu 12 guest-startfail baseline untested test-armhf-armhf-xl-midway 12 guest-start fail baseline untested test-armhf-armhf-xl-rtds 12 guest-start fail baseline untested test-armhf-armhf-xl 12 guest-start fail baseline untested test-amd64-amd64-qemuu-nested-intel 14 xen-boot/l1 fail baseline untested test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-localmigrate/x10 fail baseline untested test-armhf-armhf-xl-vhd 10 debian-di-install fail baseline untested test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail baseline untested test-armhf-armhf-libvirt-raw 10 debian-di-install fail baseline untested test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail baseline untested test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-installfail never pass 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-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-i386-libvirt 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-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass version targeted for testing: qemuu27e757e29cc79f3f104d2a84d17cdb3b4c11c8ff baseline version: qemuu38e83a71d02e026d4a6d0ab1ef9855c4924c2c68 Last test of basis74598 2018-04-13 20:46:40 Z 10 days Testing same since74638 2018-04-24 11:46:01 Z0 days1 attempts People who touched revisions under test: Alex Bennée Bastian Koppelmann Eduardo Habkost Emilio G. Cota Jason Wang Laurent Vivier Marc-André Lureau Max Reitz Michael S. Tsirkin Michael Tokarev Pavel Dovgalyuk Peter Maydell Philippe Mathieu-Daudé Richard Henderson Vladimir Sementsov-Ogievskiy Wanpeng Li jobs: build-amd64-xsm pass build-armhf-xsm pass build-i386-xsm pass build-amd64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-xl pass test-armhf-armhf-xl fail test-amd64-i386-xl pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsmpass test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm pass test-amd64-amd64-libvirt-xsm pass test-armhf-armhf-libvirt-xsm fail test-amd64-i386-libvirt-xsm pass test-amd64-amd64-xl-xsm pass test-armhf-armhf-xl-xsm fail test-amd64-i386-xl-xsm
Re: [Xen-devel] [Qemu-devel] [PATCH 09/16] os-posix: cleanup: Replace fprintfs with error_report in change_process_uid
On 24 April 2018 at 18:58, Ian Jackson wrote: > I'm going to be editing this function and it makes sense to clean up > this style problem in advance. > > Signed-off-by: Ian Jackson > CC: Paolo Bonzini > CC: Markus Armbruster > CC: Daniel P. Berrange > CC: Michael Tokarev > --- Reviewed-by: Peter Maydell thanks -- PMM ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-4.11] x86/SVM: Fix intercepted {RD, WR}MSR for the SYS{CALL, ENTER} MSRs
By default, the SYSCALL MSRs are not intercepted, and accesses are completed
by hardware. The SYSENTER MSRs are intercepted for cross-vendor
purposes (albeit needlessly in the common case), and are fully emulated.
However, {RD,WR}MSR instructions which happen to be emulated (FEP,
introspection, or older versions of Xen which intercepted #UD), or when the
MSRs are explicitly intercepted (introspection), will be completed
incorrectly.
svm_msr_read_intercept() appears to return the correct values, but only
because of the default read-everything case (which is going to disappear), and
that in vcpu context, hardware should have the guest values in context.
Update the read path to explicitly sync the VMCB and complete the accesses,
rather than falling all the way through to the default case.
svm_msr_write_intercept() silently discard all updates. Synchronise the VMCB
for all applicable MSRs, and implement suitable checks. The actual behaviour
of AMD hardware is to truncate the SYSENTER and SFMASK MSRs at 32 bits, but
this isn't implemented yet to remain compatible with the cross-vendor case.
Drop one bit of trailing whitespace while modifing this area of the code.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Juergen Gross
CC: Boris Ostrovsky
CC: Suravee Suthikulpanit
CC: Brian Woods
Juergen: As this patch probably wants backporting to the stable branches, it
probably wants to go into 4.11 at this point.
Outstanding TODOs (for someone with more tuits than me right now, or me when I
move this to the new MSR infrastructure):
* Don't intercept the SYSENTER MSRs in the common case.
* CPUID check for the long-mode only MSRs.
* Proper cross-vendor behaviour.
---
xen/arch/x86/hvm/svm/svm.c | 111 +
1 file changed, 102 insertions(+), 9 deletions(-)
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index c761645..2e42edf 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1883,6 +1883,22 @@ static int svm_msr_read_intercept(unsigned int msr,
uint64_t *msr_content)
switch ( msr )
{
case MSR_IA32_SYSENTER_CS:
+case MSR_IA32_SYSENTER_ESP:
+case MSR_IA32_SYSENTER_EIP:
+case MSR_STAR:
+case MSR_LSTAR:
+case MSR_CSTAR:
+case MSR_SYSCALL_MASK:
+case MSR_FS_BASE:
+case MSR_GS_BASE:
+case MSR_SHADOW_GS_BASE:
+svm_sync_vmcb(v);
+break;
+}
+
+switch ( msr )
+{
+case MSR_IA32_SYSENTER_CS:
*msr_content = v->arch.hvm_svm.guest_sysenter_cs;
break;
case MSR_IA32_SYSENTER_ESP:
@@ -1892,6 +1908,34 @@ static int svm_msr_read_intercept(unsigned int msr,
uint64_t *msr_content)
*msr_content = v->arch.hvm_svm.guest_sysenter_eip;
break;
+case MSR_STAR:
+*msr_content = vmcb->star;
+break;
+
+case MSR_LSTAR:
+*msr_content = vmcb->lstar;
+break;
+
+case MSR_CSTAR:
+*msr_content = vmcb->cstar;
+break;
+
+case MSR_SYSCALL_MASK:
+*msr_content = vmcb->sfmask;
+break;
+
+case MSR_FS_BASE:
+*msr_content = vmcb->fs.base;
+break;
+
+case MSR_GS_BASE:
+*msr_content = vmcb->gs.base;
+break;
+
+case MSR_SHADOW_GS_BASE:
+*msr_content = vmcb->kerngsbase;
+break;
+
case MSR_IA32_MCx_MISC(4): /* Threshold register */
case MSR_F10_MC4_MISC1 ... MSR_F10_MC4_MISC3:
/*
@@ -2020,32 +2064,81 @@ static int svm_msr_write_intercept(unsigned int msr,
uint64_t msr_content)
int ret, result = X86EMUL_OKAY;
struct vcpu *v = current;
struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
-int sync = 0;
+bool sync = false;
switch ( msr )
{
case MSR_IA32_SYSENTER_CS:
case MSR_IA32_SYSENTER_ESP:
case MSR_IA32_SYSENTER_EIP:
-sync = 1;
-break;
-default:
+case MSR_STAR:
+case MSR_LSTAR:
+case MSR_CSTAR:
+case MSR_SYSCALL_MASK:
+case MSR_FS_BASE:
+case MSR_GS_BASE:
+case MSR_SHADOW_GS_BASE:
+sync = true;
break;
}
if ( sync )
-svm_sync_vmcb(v);
+svm_sync_vmcb(v);
switch ( msr )
{
+case MSR_IA32_SYSENTER_ESP:
+case MSR_IA32_SYSENTER_EIP:
+case MSR_LSTAR:
+case MSR_CSTAR:
+case MSR_FS_BASE:
+case MSR_GS_BASE:
+case MSR_SHADOW_GS_BASE:
+if ( !is_canonical_address(msr_content) )
+goto gpf;
+
+switch ( msr )
+{
+case MSR_IA32_SYSENTER_ESP:
+vmcb->sysenter_esp = v->arch.hvm_svm.guest_sysenter_esp =
msr_content;
+break;
+
+case MSR_IA32_SYSENTER_EIP:
+vmcb->sysenter_eip = v->arch.hvm_svm.guest_sysenter_eip =
msr_content;
+break;
+
+case MSR_LSTAR:
+vmcb->lstar = msr_content;
+break;
+
+case MSR_CSTAR:
+vmcb->cstar = msr_content;
+break;
+
+case MS
[Xen-devel] [ovmf test] 122362: all pass - PUSHED
flight 122362 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/122362/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf ee4dc24f57c32a445e7c747396c9bfbd8b221568 baseline version: ovmf 55f67014d7b4a1228754313917ccca5539764802 Last test of basis 122346 2018-04-17 07:16:34 Z7 days Testing same since 122362 2018-04-23 11:18:13 Z1 days1 attempts People who touched revisions under test: Ard Biesheuvel Carsey, Jaben Carsey, Jaben Dandan Bi Evan Lloyd Hao Wu Jaben Carsey Laszlo Ersek Liming Gao Ruiyu Ni Sami Mujawar Star Zeng Yunhua Feng jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/osstest/ovmf.git 55f67014d7..ee4dc24f57 ee4dc24f57c32a445e7c747396c9bfbd8b221568 -> xen-tested-master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 14/16] os-posix: cleanup: Replace fprintf with error_report in remaining call sites
Signed-off-by: Ian Jackson
CC: Paolo Bonzini
CC: Markus Armbruster
CC: Daniel P. Berrange
CC: Michael Tokarev
Reviewed-by: Philippe Mathieu-Daudé
---
v8: Remove one remaining spurious "\n"
v7: New patch
---
os-posix.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/os-posix.c b/os-posix.c
index 0f59566..a2ba50d 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -129,7 +129,7 @@ void os_set_proc_name(const char *s)
exit(1);
}
#else
-fprintf(stderr, "Change of process name not supported by your OS\n");
+error_report("Change of process name not supported by your OS");
exit(1);
#endif
}
@@ -243,7 +243,7 @@ static void change_root(void)
{
if (chroot_dir) {
if (chroot(chroot_dir) < 0) {
-fprintf(stderr, "chroot failed\n");
+error_report("chroot failed");
exit(1);
}
if (chdir("/")) {
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 04/16] xen: restrict: use xentoolcore_restrict_all
And insist that it works.
Drop individual use of xendevicemodel_restrict and
xenforeignmemory_restrict. These are not actually effective in this
version of qemu, because qemu has a large number of fds open onto
various Xen control devices.
The restriction arrangements are still not right, because the
restriction needs to be done very late - after qemu has opened all of
its control fds.
xentoolcore_restrict_all and xentoolcore.h are available in Xen 4.10
and later, only. Provide a compatibility stub. And drop the
compatibility stubs for the old functions.
Signed-off-by: Ian Jackson
Reviewed-by: Anthony PERARD
Acked-by: Stefano Stabellini
---
v2: Modify the compatibility code, too.
Bump this patch ahead of "defer call to xen_restrict until running"
Retain call to xentoolcore_restrict_all
---
include/hw/xen/xen_common.h | 46 +++--
1 file changed, 11 insertions(+), 35 deletions(-)
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 64a978e..1766bb9 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -91,6 +91,16 @@ static inline void
*xenforeignmemory_map2(xenforeignmemory_handle *h,
return xenforeignmemory_map(h, dom, prot, pages, arr, err);
}
+static inline int xentoolcore_restrict_all(domid_t domid)
+{
+errno = ENOTTY;
+return -1;
+}
+
+#else /* CONFIG_XEN_CTRL_INTERFACE_VERSION >= 41000 */
+
+#include
+
#endif
#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 40900
@@ -218,20 +228,6 @@ static inline int xendevicemodel_set_mem_type(
return xc_hvm_set_mem_type(dmod, domid, mem_type, first_pfn, nr);
}
-static inline int xendevicemodel_restrict(
-xendevicemodel_handle *dmod, domid_t domid)
-{
-errno = ENOTTY;
-return -1;
-}
-
-static inline int xenforeignmemory_restrict(
-xenforeignmemory_handle *fmem, domid_t domid)
-{
-errno = ENOTTY;
-return -1;
-}
-
#else /* CONFIG_XEN_CTRL_INTERFACE_VERSION >= 40900 */
#undef XC_WANT_COMPAT_DEVICEMODEL_API
@@ -290,28 +286,8 @@ static inline int xen_modified_memory(domid_t domid,
uint64_t first_pfn,
static inline int xen_restrict(domid_t domid)
{
int rc;
-
-/* Attempt to restrict devicemodel operations */
-rc = xendevicemodel_restrict(xen_dmod, domid);
+rc = xentoolcore_restrict_all(domid);
trace_xen_domid_restrict(rc ? errno : 0);
-
-if (rc < 0) {
-/*
- * If errno is ENOTTY then restriction is not implemented so
- * there's no point in trying to restrict other types of
- * operation, but it should not be treated as a failure.
- */
-if (errno == ENOTTY) {
-return 0;
-}
-
-return rc;
-}
-
-/* Restrict foreignmemory operations */
-rc = xenforeignmemory_restrict(xen_fmem, domid);
-trace_xen_domid_restrict(rc ? errno : 0);
-
return rc;
}
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v8 00/16] xen: xen-domid-restrict improvements
This series provides necessary support for running qemu as a Xen device model without power equivalent to root. In particular, it makes -xen-domid-restrict effective. Compared to v8, it addresses review comments. 01/16 checkpatch: Add xendevicemodel_handle to the list of r 02/16 AccelClass: Introduce accel_setup_post * a 03/16 xen: link against xentoolcore a 04/16 xen: restrict: use xentoolcore_restrict_all * a 05/16 xen: defer call to xen_restrict until just before a 06/16 xen: destroy_hvm_domain: Move reason into a variable a 07/16 xen: move xc_interface compatibility fallback further r 08/16 xen: destroy_hvm_domain: Try xendevicemodel_shutdown 09/16 os-posix: cleanup: Replace fprintfs with error_report in change_... r 10/16 os-posix: Provide new -runas : facility a 11/16 xen: Use newly added dmops for mapping VGA memory a 12/16 xen: Remove now-obsolete xen_xc_domain_add_to_physmap a 13/16 xen: Expect xenstore write to fail when restricted *r 14/16 os-posix: cleanup: Replace fprintf with error_report in remaining r 15/16 os-posix: cleanup: Replace perror with error_report *16/16 configure: do_compiler: Dump some extra info under bash r = reviewed (by someone other than me) a = acked * = amended patch Thanks for your attention. Ian. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 11/16] xen: Use newly added dmops for mapping VGA memory
From: Ross Lagerwall
Xen unstable (to be in 4.11) has two new dmops, relocate_memory and
pin_memory_cacheattr. Use these to set up the VGA memory, replacing the
previous calls to libxc. This allows the VGA console to work properly
when QEMU is running restricted (-xen-domid-restrict).
Wrapper functions are provided to allow QEMU to work with older versions
of Xen.
Tweak the error handling while making this change:
* Report pin_memory_cacheattr errors.
* Report errors even when DEBUG_HVM is not set. This is useful for
trying to understand why VGA is not working, since otherwise it just
fails silently.
* Fix the return values when an error occurs. The functions now
consistently return -1 and set errno.
CC: Ian Jackson
Signed-off-by: Ross Lagerwall
Reviewed-by: Ian Jackson
Signed-off-by: Ian Jackson
Acked-by: Anthony PERARD
---
v6.1: Fix printf formats to match types in error_report messages
Fix spurious \n in error_report messages
Fix { } style issue
v6: New patch in this version of the series
---
configure | 19 +
hw/i386/xen/xen-hvm.c | 50 -
include/hw/xen/xen_common.h | 32 +
3 files changed, 78 insertions(+), 23 deletions(-)
diff --git a/configure b/configure
index 5cf9dde..aa35aef 100755
--- a/configure
+++ b/configure
@@ -2221,6 +2221,25 @@ EOF
# Xen unstable
elif
cat > $TMPC <
+int main(void) {
+ xendevicemodel_handle *xd;
+
+ xd = xendevicemodel_open(0, 0);
+ xendevicemodel_pin_memory_cacheattr(xd, 0, 0, 0, 0);
+
+ return 0;
+}
+EOF
+compile_prog "" "$xen_libs -lxendevicemodel $xen_stable_libs
-lxentoolcore"
+ then
+ xen_stable_libs="-lxendevicemodel $xen_stable_libs -lxentoolcore"
+ xen_ctrl_version=41100
+ xen=yes
+elif
+cat > $TMPC <
#include
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index fb727bc..caa563b 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -347,7 +347,7 @@ static int xen_add_to_physmap(XenIOState *state,
MemoryRegion *mr,
hwaddr offset_within_region)
{
-unsigned long i = 0;
+unsigned long nr_pages;
int rc = 0;
XenPhysmap *physmap = NULL;
hwaddr pfn, start_gpfn;
@@ -396,22 +396,26 @@ go_physmap:
pfn = phys_offset >> TARGET_PAGE_BITS;
start_gpfn = start_addr >> TARGET_PAGE_BITS;
-for (i = 0; i < size >> TARGET_PAGE_BITS; i++) {
-unsigned long idx = pfn + i;
-xen_pfn_t gpfn = start_gpfn + i;
-
-rc = xen_xc_domain_add_to_physmap(xen_xc, xen_domid, XENMAPSPACE_gmfn,
idx, gpfn);
-if (rc) {
-DPRINTF("add_to_physmap MFN %"PRI_xen_pfn" to PFN %"
-PRI_xen_pfn" failed: %d (errno: %d)\n", idx, gpfn, rc,
errno);
-return -rc;
-}
+nr_pages = size >> TARGET_PAGE_BITS;
+rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, nr_pages, pfn,
+start_gpfn);
+if (rc) {
+int saved_errno = errno;
+
+error_report("relocate_memory %lu pages from GFN %"HWADDR_PRIx
+ " to GFN %"HWADDR_PRIx" failed: %s",
+ nr_pages, pfn, start_gpfn, strerror(saved_errno));
+errno = saved_errno;
+return -1;
}
-xc_domain_pin_memory_cacheattr(xen_xc, xen_domid,
+rc = xendevicemodel_pin_memory_cacheattr(xen_dmod, xen_domid,
start_addr >> TARGET_PAGE_BITS,
(start_addr + size - 1) >> TARGET_PAGE_BITS,
XEN_DOMCTL_MEM_CACHEATTR_WB);
+if (rc) {
+error_report("pin_memory_cacheattr failed: %s", strerror(errno));
+}
return xen_save_physmap(state, physmap);
}
@@ -419,7 +423,6 @@ static int xen_remove_from_physmap(XenIOState *state,
hwaddr start_addr,
ram_addr_t size)
{
-unsigned long i = 0;
int rc = 0;
XenPhysmap *physmap = NULL;
hwaddr phys_offset = 0;
@@ -438,16 +441,17 @@ static int xen_remove_from_physmap(XenIOState *state,
size >>= TARGET_PAGE_BITS;
start_addr >>= TARGET_PAGE_BITS;
phys_offset >>= TARGET_PAGE_BITS;
-for (i = 0; i < size; i++) {
-xen_pfn_t idx = start_addr + i;
-xen_pfn_t gpfn = phys_offset + i;
-
-rc = xen_xc_domain_add_to_physmap(xen_xc, xen_domid, XENMAPSPACE_gmfn,
idx, gpfn);
-if (rc) {
-fprintf(stderr, "add_to_physmap MFN %"PRI_xen_pfn" to PFN %"
-PRI_xen_pfn" failed: %d (errno: %d)\n", idx, gpfn, rc,
errno);
-return -rc;
-}
+rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, size, start_addr,
+phys_offset);
+if (rc) {
+int saved_errno = errno;
+
+error_report("relocate
[Xen-devel] [PATCH 07/16] xen: move xc_interface compatibility fallback further up the file
We are going to want to use the dummy xendevicemodel_handle type in
new stub functions in the CONFIG_XEN_CTRL_INTERFACE_VERSION < 41000
section. So we need to provide that definition, or (as applicable)
include the appropriate header, earlier in the file.
(Ideally the newer compatibility layers would be at the bottom of the
file, so that they can naturally benefit from the compatibility layers
for earlier version. But that's rather too much for this series.)
No functional change.
Signed-off-by: Ian Jackson
Acked-by: Anthony PERARD
Acked-by: Stefano Stabellini
---
v2: New patch in v2 of the series
---
include/hw/xen/xen_common.h | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 1766bb9..60c4ebb 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -78,6 +78,17 @@ static inline void *xenforeignmemory_map(xc_interface *h,
uint32_t dom,
extern xenforeignmemory_handle *xen_fmem;
+#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 40900
+
+typedef xc_interface xendevicemodel_handle;
+
+#else /* CONFIG_XEN_CTRL_INTERFACE_VERSION >= 40900 */
+
+#undef XC_WANT_COMPAT_DEVICEMODEL_API
+#include
+
+#endif
+
#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 41000
#define XEN_COMPAT_PHYSMAP
@@ -105,8 +116,6 @@ static inline int xentoolcore_restrict_all(domid_t domid)
#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 40900
-typedef xc_interface xendevicemodel_handle;
-
static inline xendevicemodel_handle *xendevicemodel_open(
struct xentoollog_logger *logger, unsigned int open_flags)
{
@@ -228,11 +237,6 @@ static inline int xendevicemodel_set_mem_type(
return xc_hvm_set_mem_type(dmod, domid, mem_type, first_pfn, nr);
}
-#else /* CONFIG_XEN_CTRL_INTERFACE_VERSION >= 40900 */
-
-#undef XC_WANT_COMPAT_DEVICEMODEL_API
-#include
-
#endif
extern xendevicemodel_handle *xen_dmod;
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 05/16] xen: defer call to xen_restrict until just before os_setup_post
We need to restrict *all* the control fds that qemu opens. Looking in
/proc/PID/fd shows there are many; their allocation seems scattered
throughout Xen support code in qemu.
We must postpone the restrict call until roughly the same time as qemu
changes its uid, chroots (if applicable), and so on.
There doesn't seem to be an appropriate hook already. The RunState
change hook fires at different times depending on exactly what mode
qemu is operating in.
And it appears that no-one but the Xen code wants a hook at this phase
of execution. So, introduce a bare call to a new function
xen_setup_post, just before os_setup_post. Also provide the
appropriate stub for when Xen compilation is disabled.
We do the restriction before rather than after os_setup_post, because
xen_restrict may need to open /dev/null, and os_setup_post might have
called chroot.
Currently this does not work with migration, because when running as
the Xen device model qemu needs to signal to the toolstack that it is
ready. It currently does this using xenstore, and for incoming
migration (but not for ordinary startup) that happens after
os_setup_post.
It is correct that this happens late: we want the incoming migration
stream to be processed by a restricted qemu. The fix for this will be
to do the startup notification a different way, without using
xenstore. (QMP is probably a reasonable choice.)
So for now this restriction feature cannot be used in conjunction with
migration. (Note that this is not a regression in this patch, because
previously the -xen-restrict-domid call was, in fact, simply
ineffective!) We will revisit this in the Xen 4.11 release cycle.
Signed-off-by: Ian Jackson
CC: Paolo Bonzini (maintainer:X86)
CC: Richard Henderson (maintainer:X86)
CC: Eduardo Habkost (maintainer:X86)
CC: Michael S. Tsirkin (supporter:PC)
Acked-by: Anthony PERARD
---
v8: Drop now-redundant hunk patching stubs/xen-hvm.c
v7: Use new AccelClass setup_post hook, rather than ad-hoc call
in vl.c.
v5: Discuss problems with migration startup notification
in the commit message.
v3: Do xen_setup_post just before, not just after, os_setup_post,
to improve interaction with chroot. Thanks to Ross Lagerwall.
---
hw/i386/xen/xen-hvm.c | 8
hw/xen/xen-common.c | 14 ++
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index f24b7d4..9c3b6b3 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -1254,14 +1254,6 @@ void xen_hvm_init(PCMachineState *pcms, MemoryRegion
**ram_memory)
goto err;
}
-if (xen_domid_restrict) {
-rc = xen_restrict(xen_domid);
-if (rc < 0) {
-error_report("failed to restrict: error %d", errno);
-goto err;
-}
-}
-
xen_create_ioreq_server(xen_domid, &state->ioservid);
state->exit.notify = xen_exit_notifier;
diff --git a/hw/xen/xen-common.c b/hw/xen/xen-common.c
index 83099dd..454777c 100644
--- a/hw/xen/xen-common.c
+++ b/hw/xen/xen-common.c
@@ -117,6 +117,19 @@ static void xen_change_state_handler(void *opaque, int
running,
}
}
+static void xen_setup_post(MachineState *ms, AccelState *accel)
+{
+int rc;
+
+if (xen_domid_restrict) {
+rc = xen_restrict(xen_domid);
+if (rc < 0) {
+perror("xen: failed to restrict");
+exit(1);
+}
+}
+}
+
static int xen_init(MachineState *ms)
{
xen_xc = xc_interface_open(0, 0, 0);
@@ -165,6 +178,7 @@ static void xen_accel_class_init(ObjectClass *oc, void
*data)
AccelClass *ac = ACCEL_CLASS(oc);
ac->name = "Xen";
ac->init_machine = xen_init;
+ac->setup_post = xen_setup_post;
ac->allowed = &xen_allowed;
ac->global_props = xen_compat_props;
}
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 06/16] xen: destroy_hvm_domain: Move reason into a variable
We are going to want to reuse this.
No functional change.
Signed-off-by: Ian Jackson
Reviewed-by: Anthony PERARD
Acked-by: Stefano Stabellini
---
hw/i386/xen/xen-hvm.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 9c3b6b3..3590d99 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -1387,12 +1387,13 @@ void destroy_hvm_domain(bool reboot)
xc_interface *xc_handle;
int sts;
+unsigned int reason = reboot ? SHUTDOWN_reboot : SHUTDOWN_poweroff;
+
xc_handle = xc_interface_open(0, 0, 0);
if (xc_handle == NULL) {
fprintf(stderr, "Cannot acquire xenctrl handle\n");
} else {
-sts = xc_domain_shutdown(xc_handle, xen_domid,
- reboot ? SHUTDOWN_reboot : SHUTDOWN_poweroff);
+sts = xc_domain_shutdown(xc_handle, xen_domid, reason);
if (sts != 0) {
fprintf(stderr, "xc_domain_shutdown failed to issue %s, "
"sts %d, %s\n", reboot ? "reboot" : "poweroff",
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 15/16] os-posix: cleanup: Replace perror with error_report
perror() is defined to fprintf(stderr,...). HACKING says
fprintf(stderr,...) is wrong. So perror() is too.
Signed-off-by: Ian Jackson
CC: Paolo Bonzini
CC: Markus Armbruster
CC: Daniel P. Berrange
CC: Michael Tokarev
CC: Alistair Francis
Reviewed-by: Philippe Mathieu-Daudé
---
v7: New patch
---
os-posix.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/os-posix.c b/os-posix.c
index a2ba50d..24eb700 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -125,7 +125,7 @@ void os_set_proc_name(const char *s)
/* Could rewrite argv[0] too, but that's a bit more complicated.
This simple way is enough for `top'. */
if (prctl(PR_SET_NAME, name)) {
-perror("unable to change process name");
+error_report("unable to change process name: %s", strerror(errno));
exit(1);
}
#else
@@ -247,7 +247,7 @@ static void change_root(void)
exit(1);
}
if (chdir("/")) {
-perror("not able to chdir to /");
+error_report("not able to chdir to /: %s", strerror(errno));
exit(1);
}
}
@@ -309,7 +309,7 @@ void os_setup_post(void)
if (daemonize) {
if (chdir("/")) {
-perror("not able to chdir to /");
+error_report("not able to chdir to /: %s", strerror(errno));
exit(1);
}
TFR(fd = qemu_open("/dev/null", O_RDWR));
@@ -383,7 +383,7 @@ int os_mlock(void)
ret = mlockall(MCL_CURRENT | MCL_FUTURE);
if (ret < 0) {
-perror("mlockall");
+error_report("mlockall: %s", strerror(errno));
}
return ret;
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 03/16] xen: link against xentoolcore
From: Anthony PERARD
Xen libraries in 4.10 include a new xentoolcore library. This
contains the xentoolcore_restrict_all function which we are about to
want to use.
Signed-off-by: Ian Jackson
Acked-by: Stefano Stabellini
---
v8: In pkg-config branch, add xentoolcore conditionally.
This cope with Xen 4.9 found via pkg-config.
v5: More truthful commit message.
---
configure | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/configure b/configure
index 0a19b03..5cf9dde 100755
--- a/configure
+++ b/configure
@@ -2189,6 +2189,9 @@ if test "$xen" != "no" ; then
xen=yes
xen_pc="xencontrol xenstore xenguest xenforeignmemory xengnttab"
xen_pc="$xen_pc xenevtchn xendevicemodel"
+if $pkg_config --exists xentoolcore; then
+ xen_pc="$xen_pc xentoolcore"
+fi
QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags $xen_pc)"
libs_softmmu="$($pkg_config --libs $xen_pc) $libs_softmmu"
LDFLAGS="$($pkg_config --libs $xen_pc) $LDFLAGS"
@@ -2220,18 +2223,20 @@ EOF
cat > $TMPC <
+#include
int main(void) {
xenforeignmemory_handle *xfmem;
xfmem = xenforeignmemory_open(0, 0);
xenforeignmemory_map2(xfmem, 0, 0, 0, 0, 0, 0, 0);
+ xentoolcore_restrict_all(0);
return 0;
}
EOF
-compile_prog "" "$xen_libs -lxendevicemodel $xen_stable_libs"
+compile_prog "" "$xen_libs -lxendevicemodel $xen_stable_libs
-lxentoolcore"
then
- xen_stable_libs="-lxendevicemodel $xen_stable_libs"
+ xen_stable_libs="-lxendevicemodel $xen_stable_libs -lxentoolcore"
xen_ctrl_version=41000
xen=yes
elif
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 02/16] AccelClass: Introduce accel_setup_post
This is called just before os_setup_post. Currently none of the
accelerators provide this hook, but the Xen one is going to provide
one in a moment.
Signed-off-by: Ian Jackson
Reviewed-by: Eduardo Habkost
---
v7: New patch in this version of the series
---
accel/accel.c | 9 +
include/sysemu/accel.h | 3 +++
vl.c | 1 +
3 files changed, 13 insertions(+)
diff --git a/accel/accel.c b/accel/accel.c
index 93e2434..9cfab11 100644
--- a/accel/accel.c
+++ b/accel/accel.c
@@ -126,6 +126,15 @@ void accel_register_compat_props(AccelState *accel)
register_compat_props_array(class->global_props);
}
+void accel_setup_post(MachineState *ms)
+{
+AccelState *accel = ms->accelerator;
+AccelClass *acc = ACCEL_GET_CLASS(accel);
+if (acc->setup_post) {
+acc->setup_post(ms, accel);
+}
+}
+
static void register_accel_types(void)
{
type_register_static(&accel_type);
diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h
index 5a632ce..637358f 100644
--- a/include/sysemu/accel.h
+++ b/include/sysemu/accel.h
@@ -40,6 +40,7 @@ typedef struct AccelClass {
const char *name;
int (*available)(void);
int (*init_machine)(MachineState *ms);
+void (*setup_post)(MachineState *ms, AccelState *accel);
bool *allowed;
/*
* Array of global properties that would be applied when specific
@@ -68,5 +69,7 @@ extern unsigned long tcg_tb_size;
void configure_accelerator(MachineState *ms);
/* Register accelerator specific global properties */
void accel_register_compat_props(AccelState *accel);
+/* Called just before os_setup_post (ie just before drop OS privs) */
+void accel_setup_post(MachineState *ms);
#endif
diff --git a/vl.c b/vl.c
index fce1fd1..36c5bd4 100644
--- a/vl.c
+++ b/vl.c
@@ -4729,6 +4729,7 @@ int main(int argc, char **argv, char **envp)
vm_start();
}
+accel_setup_post(current_machine);
os_setup_post();
main_loop();
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 10/16] os-posix: Provide new -runas : facility
This allows the caller to specify a uid and gid to use, even if there
is no corresponding password entry. This will be useful in certain
Xen configurations.
We don't support just -runas because: (i) deprivileging without
calling setgroups would be ineffective (ii) given only a uid we don't
know what gid we ought to use (since uids may eppear in multiple
passwd file entries with different gids).
Signed-off-by: Ian Jackson
Reviewed-by: Anthony PERARD
CC: Paolo Bonzini
CC: Markus Armbruster
CC: Daniel P. Berrange
CC: Michael Tokarev
Reviewed-by: Markus Armbruster
---
v7: Be much more explicit about legal combinations of user_{pwd,uid,gid}.
Rely more on qemu_stroul being sane, dropping pointless
pre-assignments of errno and lv.
Retain optarg in error message about -runas argument.
Rebase over introduction of error_report in change_process_uid.
v6.1: Fix constness of qemu_strtoul end pointer parameter.
v6: Use qemu_strtoul for the first strtoul.
Use error_report rather than fprintf to print usage error message.
Fix an error message which still referred to . rather than :
v5: Use : rather than . to separate uid from gid
v4: Changed to reuse option -runas
v3: Error messages fixed. Thanks to Peter Maydell and Ross Lagerwall.
---
os-posix.c | 77 -
qemu-options.hx | 3 ++-
2 files changed, 67 insertions(+), 13 deletions(-)
diff --git a/os-posix.c b/os-posix.c
index 560db95..0f59566 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -41,7 +41,14 @@
#include
#endif
-static struct passwd *user_pwd;
+/*
+ * Must set all three of these at once.
+ * Legal combinations are unset by name by uid
+ */
+static struct passwd *user_pwd;/* NULL non-NULL NULL */
+static uid_t user_uid = (uid_t)-1; /* -1 -1>=0*/
+static gid_t user_gid = (gid_t)-1; /* -1 -1>=0*/
+
static const char *chroot_dir;
static int daemonize;
static int daemon_pipe;
@@ -127,6 +134,33 @@ void os_set_proc_name(const char *s)
#endif
}
+
+static bool os_parse_runas_uid_gid(const char *optarg)
+{
+unsigned long lv;
+const char *ep;
+uid_t got_uid;
+gid_t got_gid;
+int rc;
+
+rc = qemu_strtoul(optarg, &ep, 0, &lv);
+got_uid = lv; /* overflow here is ID in C99 */
+if (rc || *ep != ':' || got_uid != lv || got_uid == (uid_t)-1) {
+return false;
+}
+
+rc = qemu_strtoul(ep + 1, 0, 0, &lv);
+got_gid = lv; /* overflow here is ID in C99 */
+if (rc || got_gid != lv || got_gid == (gid_t)-1) {
+return false;
+}
+
+user_pwd = NULL;
+user_uid = got_uid;
+user_gid = got_gid;
+return true;
+}
+
/*
* Parse OS specific command line options.
* return 0 if option handled, -1 otherwise
@@ -144,8 +178,13 @@ void os_parse_cmd_args(int index, const char *optarg)
#endif
case QEMU_OPTION_runas:
user_pwd = getpwnam(optarg);
-if (!user_pwd) {
-fprintf(stderr, "User \"%s\" doesn't exist\n", optarg);
+if (user_pwd) {
+user_uid = -1;
+user_gid = -1;
+} else if (!os_parse_runas_uid_gid(optarg)) {
+error_report("User \"%s\" doesn't exist"
+ " (and is not :)",
+ optarg);
exit(1);
}
break;
@@ -165,18 +204,32 @@ void os_parse_cmd_args(int index, const char *optarg)
static void change_process_uid(void)
{
-if (user_pwd) {
-if (setgid(user_pwd->pw_gid) < 0) {
-error_report("Failed to setgid(%d)", user_pwd->pw_gid);
+assert((user_uid == (uid_t)-1) || user_pwd == NULL);
+assert((user_uid == (uid_t)-1) ==
+ (user_gid == (gid_t)-1));
+
+if (user_pwd || user_uid != (uid_t)-1) {
+gid_t intended_gid = user_pwd ? user_pwd->pw_gid : user_gid;
+uid_t intended_uid = user_pwd ? user_pwd->pw_uid : user_uid;
+if (setgid(intended_gid) < 0) {
+error_report("Failed to setgid(%d)", intended_gid);
exit(1);
}
-if (initgroups(user_pwd->pw_name, user_pwd->pw_gid) < 0) {
-error_report("Failed to initgroups(\"%s\", %d)",
- user_pwd->pw_name, user_pwd->pw_gid);
-exit(1);
+if (user_pwd) {
+if (initgroups(user_pwd->pw_name, user_pwd->pw_gid) < 0) {
+error_report("Failed to initgroups(\"%s\", %d)",
+user_pwd->pw_name, user_pwd->pw_gid);
+exit(1);
+}
+} else {
+if (setgroups(1, &user_gid) < 0) {
+error_report("Failed to setgroups(1, [%d])",
+user_gid);
+exit(1);
+}
}
-if (setuid(user_pwd->pw_uid) < 0) {
-error_report("Failed to setuid(%d)", user_pwd->pw_uid);
+if (setuid(intended_uid) < 0) {
+error_report("Failed to setuid(%d)",
[Xen-devel] [PATCH 13/16] xen: Expect xenstore write to fail when restricted
From: Ross Lagerwall
Saving the current state to xenstore may fail when running restricted
(in particular, after a migration). Therefore, don't report the error or
exit when running restricted. Toolstacks that want to allow running
QEMU restricted should instead make use of QMP events to listen for
state changes.
CC: Ian Jackson
Signed-off-by: Ross Lagerwall
Reviewed-by: Ian Jackson
Acked-by: Anthony PERARD
---
v6: New patch in this version of the series
---
hw/xen/xen-common.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/hw/xen/xen-common.c b/hw/xen/xen-common.c
index 454777c..6ec14c7 100644
--- a/hw/xen/xen-common.c
+++ b/hw/xen/xen-common.c
@@ -101,7 +101,12 @@ static void xenstore_record_dm_state(struct xs_handle *xs,
const char *state)
}
snprintf(path, sizeof (path), "device-model/%u/state", xen_domid);
-if (!xs_write(xs, XBT_NULL, path, state, strlen(state))) {
+/*
+ * This call may fail when running restricted so don't make it fatal in
+ * that case. Toolstacks should instead use QMP to listen for state
changes.
+ */
+if (!xs_write(xs, XBT_NULL, path, state, strlen(state)) &&
+!xen_domid_restrict) {
error_report("error recording dm state");
exit(1);
}
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 01/16] checkpatch: Add xendevicemodel_handle to the list of types
This avoids checkpatch misparsing (as statements) long function
definitions or declarations, which sometimes start with constructs
like this:
static inline int xendevicemodel_relocate_memory(
xendevicemodel_handle *dmod, domid_t domid, ...
The type xendevicemodel_handle does not conform to Qemu CODING_STYLE,
which would suggest CamelCase. However, it is a type defined by the
Xen Project in xen.git. It would be possible to introduce a typedef
to allow the qemu code to refer to it by a differently-spelled name,
but that would obfuscate more than it would clarify.
CC: Eric Blake
CC: Paolo Bonzini
CC: Daniel P. Berrange
Signed-off-by: Ian Jackson
---
v7: Added comment to commit message about why we are not renaming
this type to CamelCase.
v6.1: New patch
---
scripts/checkpatch.pl | 1 +
1 file changed, 1 insertion(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index d52207a..5b8735d 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -266,6 +266,7 @@ our @typeList = (
qr{target_(?:u)?long},
qr{hwaddr},
qr{xml${Ident}},
+ qr{xendevicemodel_handle},
);
# This can be modified by sub possible. Since it can be empty, be careful
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 16/16] configure: do_compiler: Dump some extra info under bash
This makes it much easier to find a particular thing in config.log.
We have to use the ${BASH_LINENO[*]} syntax which is a syntax error in
other shells, so test what shell we are running and use eval.
The extra output is only printed if configure is run with bash. On
systems where /bin/sh is not bash, it is necessary to say bash
./configure to get the extra debug info in the log.
Suggested-by: Eric Blake
Signed-off-by: Ian Jackson
CC: Kent R. Spillner
CC: Janosch Frank
CC: Thomas Huth
CC: Peter Maydell
CC: Paolo Bonzini
---
v8: Fix so that it actually works as intended with bash.
v6: Fix commit message wording.
v4: No longer tag this patch RFC.
---
configure | 5 +
1 file changed, 5 insertions(+)
diff --git a/configure b/configure
index aa35aef..f9ba9ea 100755
--- a/configure
+++ b/configure
@@ -60,6 +60,11 @@ do_compiler() {
# is compiler binary to execute.
local compiler="$1"
shift
+if test -n "$BASH_VERSION"; then eval '
+echo >>config.log "
+funcs: ${FUNCNAME[*]}
+lines: ${BASH_LINENO[*]}"
+'; fi
echo $compiler "$@" >> config.log
$compiler "$@" >> config.log 2>&1 || return $?
# Test passed. If this is an --enable-werror build, rerun
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 12/16] xen: Remove now-obsolete xen_xc_domain_add_to_physmap
The last user was just removed; remove this function, accordingly.
Signed-off-by: Ian Jackson
Acked-by: Anthony PERARD
---
include/hw/xen/xen_common.h | 22 --
1 file changed, 22 deletions(-)
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 2eed6fc..5f1402b 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -645,28 +645,6 @@ static inline int xen_set_ioreq_server_state(domid_t dom,
#endif
-#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 40600
-static inline int xen_xc_domain_add_to_physmap(xc_interface *xch, uint32_t
domid,
- unsigned int space,
- unsigned long idx,
- xen_pfn_t gpfn)
-{
-return xc_domain_add_to_physmap(xch, domid, space, idx, gpfn);
-}
-#else
-static inline int xen_xc_domain_add_to_physmap(xc_interface *xch, uint32_t
domid,
- unsigned int space,
- unsigned long idx,
- xen_pfn_t gpfn)
-{
-/* In Xen 4.6 rc is -1 and errno contains the error value. */
-int rc = xc_domain_add_to_physmap(xch, domid, space, idx, gpfn);
-if (rc == -1)
-return errno;
-return rc;
-}
-#endif
-
#ifdef CONFIG_XEN_PV_DOMAIN_BUILD
#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 40700
static inline int xen_domain_create(xc_interface *xc, uint32_t ssidref,
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 09/16] os-posix: cleanup: Replace fprintfs with error_report in change_process_uid
I'm going to be editing this function and it makes sense to clean up
this style problem in advance.
Signed-off-by: Ian Jackson
CC: Paolo Bonzini
CC: Markus Armbruster
CC: Daniel P. Berrange
CC: Michael Tokarev
---
os-posix.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/os-posix.c b/os-posix.c
index b9c2343..560db95 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -167,20 +167,20 @@ static void change_process_uid(void)
{
if (user_pwd) {
if (setgid(user_pwd->pw_gid) < 0) {
-fprintf(stderr, "Failed to setgid(%d)\n", user_pwd->pw_gid);
+error_report("Failed to setgid(%d)", user_pwd->pw_gid);
exit(1);
}
if (initgroups(user_pwd->pw_name, user_pwd->pw_gid) < 0) {
-fprintf(stderr, "Failed to initgroups(\"%s\", %d)\n",
-user_pwd->pw_name, user_pwd->pw_gid);
+error_report("Failed to initgroups(\"%s\", %d)",
+ user_pwd->pw_name, user_pwd->pw_gid);
exit(1);
}
if (setuid(user_pwd->pw_uid) < 0) {
-fprintf(stderr, "Failed to setuid(%d)\n", user_pwd->pw_uid);
+error_report("Failed to setuid(%d)", user_pwd->pw_uid);
exit(1);
}
if (setuid(0) != -1) {
-fprintf(stderr, "Dropping privileges failed\n");
+error_report("Dropping privileges failed");
exit(1);
}
}
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 08/16] xen: destroy_hvm_domain: Try xendevicemodel_shutdown
xc_interface_open etc. is not going to work if we have dropped
privilege, but xendevicemodel_shutdown will if everything is new
enough.
xendevicemodel_shutdown is only availabe in Xen 4.10 and later, so
provide a stub for earlier versions.
Signed-off-by: Ian Jackson
Reviewed-by: Anthony PERARD
---
v6.1: Fix { } style issue
v6: Do not print message about harmless condition in ENOTTY case.
v2: Add compatibility stub for Xen < 4.10.
Fix coding style.
---
hw/i386/xen/xen-hvm.c | 12
include/hw/xen/xen_common.h | 7 +++
2 files changed, 19 insertions(+)
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 3590d99..fb727bc 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -1386,9 +1386,21 @@ void destroy_hvm_domain(bool reboot)
{
xc_interface *xc_handle;
int sts;
+int rc;
unsigned int reason = reboot ? SHUTDOWN_reboot : SHUTDOWN_poweroff;
+if (xen_dmod) {
+rc = xendevicemodel_shutdown(xen_dmod, xen_domid, reason);
+if (!rc) {
+return;
+}
+if (errno != ENOTTY /* old Xen */) {
+perror("xendevicemodel_shutdown failed");
+}
+/* well, try the old thing then */
+}
+
xc_handle = xc_interface_open(0, 0, 0);
if (xc_handle == NULL) {
fprintf(stderr, "Cannot acquire xenctrl handle\n");
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 60c4ebb..4bd30a3 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -108,6 +108,13 @@ static inline int xentoolcore_restrict_all(domid_t domid)
return -1;
}
+static inline int xendevicemodel_shutdown(xendevicemodel_handle *dmod,
+ domid_t domid, unsigned int reason)
+{
+errno = ENOTTY;
+return -1;
+}
+
#else /* CONFIG_XEN_CTRL_INTERFACE_VERSION >= 41000 */
#include
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xtf test] 122361: all pass - PUSHED
flight 122361 xtf real [real] http://logs.test-lab.xenproject.org/osstest/logs/122361/ Perfect :-) All tests in this flight passed as required version targeted for testing: xtf c3a84a8f7cedd97b34139cb6abde62f17b9d2b1c baseline version: xtf 10bd4aec5dfd0b572269a6ba359bbf798bc98c8c Last test of basis 122281 2018-04-14 07:27:03 Z 10 days Testing same since 122361 2018-04-23 11:18:06 Z1 days1 attempts People who touched revisions under test: Andrew Cooper jobs: build-amd64-xtf pass build-amd64 pass build-amd64-pvopspass test-xtf-amd64-amd64-1 pass test-xtf-amd64-amd64-2 pass test-xtf-amd64-amd64-3 pass test-xtf-amd64-amd64-4 pass test-xtf-amd64-amd64-5 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/xtf.git 10bd4ae..c3a84a8 c3a84a8f7cedd97b34139cb6abde62f17b9d2b1c -> xen-tested-master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 02/10] xen/arm: Ignore write to GICD_ISACTIVERn registers (vgic-v2)
Hi Julien,
Thank you for the feedback, we have a v3 agreement.
On Tue, Apr 24, 2018 at 6:12 PM, Julien Grall wrote:
> Hi Mirela,
>
>
> On 04/24/2018 12:02 PM, Mirela Simonovic wrote:
>>
>> Hi Julien,
>>
>> On Mon, Apr 23, 2018 at 1:15 PM, Julien Grall
>> wrote:
>>>
>>> Hi Mirela,
>>>
>>>
>>> On 20/04/18 13:25, Mirela Simonovic wrote:
Guests attempt to write into these registers on resume (for example
Linux).
Without this patch a data abort exception will be raised to the guest.
This patch handles the write access by ignoring it, but only if the
value
to be written is zero. This should be fine because reading these
registers
is already handled as 'read as zero'.
Signed-off-by: Mirela Simonovic
---
CC: Stefano Stabellini
CC: Julien Grall
---
Changes in v2:
- Write should be ignored only if the value to be written is zero
(in v1 the write was ignored regardless of the value)
---
xen/arch/arm/vgic-v2.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index 646d1f3d12..afd3e89883 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -488,7 +488,9 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v,
mmio_info_t *info,
printk(XENLOG_G_ERR
"%pv: vGICD: unhandled word write %#"PRIregister" to
ISACTIVER%d\n",
v, r, gicd_reg - GICD_ISACTIVER);
-return 0;
+if ( r != 0 )
+return 0;
>>>
>>>
>>>
>>> It would be better to move the check before the printk. So a warning is
>>> avoided when the guest is writing 0.
>>>
>>
>> If we want to avoid printing a warning when the guest is writing 0
>> then the printk needs to be moved within the check. I guess this is
>> what you meant:
>> if ( r != 0 )
>> {
>> printk(XENLOG_G_ERR
>> "%pv: vGICD: unhandled word write %#"PRIregister" to
>> ISACTIVER%d\n",
>> v, r, gicd_reg - GICD_ISACTIVER);
>> return 0;
>> }
>> goto write_ignore_32;
>
>
> Either that or:
>
> if ( r == 0 )
> goto write_ignore_32;
>
> So you don't need to rework the code too much. Both would probably want some
> comment in the code.
>
>>
>> Please note that in the original patch where I ignored the write
>> regardless of the value I just followed how it is already done for
>> GICD_ICACTIVER.
>> For existing GICD_ICACTIVER case there is no check for the value to be
>> written and there is a warning printed.
>>
>> Not checking the value seems fine to me but why is then a warning
>> printed? Should we suppress that print as well?
>
>
> The way it is done in ICACTIVER is really fragile. The guest may think the
> active bit of the interrupt was cleared but this is not the case.
>
> It is not easy to check if the active bit is set in the current vGIC (should
> be better in the new vGIC). So it was decided to just ignore it to make
> Linux happy. The warning is here to tell the user that some may not work as
> expected.
>
> Regarding the ISACTIVER, you know that if the user write 0 none of the
> active state of the interrupts will be changed. So it is fine to avoid
> printing the warning. However, if there are one bit set then you really want
> to warn the user as the hypervisor will not probably handle it.
>
> So we want to keep the warning in both case.
>
> Cheers,
>
> --
> Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] 4.11.0 RC1 panic
Hello, I tested xen 4.11.0 rc1 with NetBSD as dom0. I could boot a NetBSD PV domU without problem, but at shutdown time (poweroff in the domU), I got a Xen panic: (XEN) Assertion 'cpu < nr_cpu_ids' failed at ...1/work/xen-4.11.0-rc1/xen/include/xen/cpumask.h:97 A xl destroy instead of poweroff gives the same result. This happens with both 32bitsPAE and 64bits domU. This doens't seem to happen with HVM domUs. Attached are a cut-n-paste of the panic, and the output of xl demsg. -- Manuel Bouyer NetBSD: 26 ans d'experience feront toujours la difference -- (XEN) Assertion 'cpu < nr_cpu_ids' failed at ...1/work/xen-4.11.0-rc1/xen/include/xen/cpumask.h:97 (XEN) [ Xen-4.11-rcnb0 x86_64 debug=y Tainted: C ] (XEN) CPU:0 (XEN) RIP:e008:[] put_page_from_l1e+0x1a3/0x1f0 (XEN) RFLAGS: 00010282 CONTEXT: hypervisor (d0v0) (XEN) rax: rbx: 8300beaad000 rcx: 0004 (XEN) rdx: 8300bf077fff rsi: 007f rdi: 8300beaad5d0 (XEN) rbp: 82d080382380 rsp: 8300bf0779b8 r8: (XEN) r9: 0200 r10: 4000 r11: 82e004207040 (XEN) r12: 830213404000 r13: 82e004207040 r14: 830213404000 (XEN) r15: 1000 cr0: 8005003b cr4: 26e0 (XEN) cr3: 00022f0f6000 cr2: 7f7ff60ce7a0 (XEN) fsb: 7f7ff7ff36c0 gsb: 80ca42c0 gss: (XEN) ds: 003f es: 003f fs: gs: ss: e010 cs: e008 (XEN) Xen code around (put_page_from_l1e+0x1a3/0x1f0): (XEN) 3b 05 cf 9c 1c 00 72 02 <0f> 0b 89 c2 83 e2 3f 48 8d 7a 01 48 c1 e7 05 48 (XEN) Xen stack trace from rsp=8300bf0779b8: (XEN)0050 820040016000 830213404000 (XEN)10ff 82d080288780 0016 0001 (XEN)0100 2401 82e0042070a0 82e004207080 (XEN)00ff 10ff 1000 82d080288e07 (XEN)002070c0 8300bf077fff 830213404000 82e0042070a0 (XEN)82e004207080 830213404000 0001 82004001e000 (XEN)0200 82d08028945f 82e004207080 (XEN)82d080288869 00210384 0206 8300bf077fff (XEN)4101 82e004207080 82e004207060 00ff (XEN)10ff 1000 82d080288e07 00010100ff22 (XEN)8300bf077fff 8300bedfc000 82e004207080 82e004207060 (XEN)830213404000 820040011000 820040011000 (XEN)82d080288540 82e004207060 830213404000 (XEN)820040011000 82d0802889eb 00210383 830213404000 (XEN)82d080280c49 6101 82e004207060 82e00411a0c0 (XEN)00ff 10ff 1000 82d080288e07 (XEN)00010136d82b 8300bf077fff 82e004207060 (XEN)82e00411a0c0 00208d06 (XEN)830213404000 82d080289146 0140 82e00411a0c0 (XEN)82d080288b95 820040001000 ffc0 (XEN) Xen call trace: (XEN)[] put_page_from_l1e+0x1a3/0x1f0 (XEN)[] free_page_type+0x210/0x790 (XEN)[] mm.c#_put_page_type+0x107/0x340 (XEN)[] mm.c#put_page_from_l2e+0xdf/0x110 (XEN)[] free_page_type+0x2f9/0x790 (XEN)[] mm.c#_put_page_type+0x107/0x340 (XEN)[] mm.c#put_page_from_l3e+0x1a0/0x1d0 (XEN)[] free_page_type+0x47b/0x790 (XEN)[] do_IRQ+0x5e9/0x630 (XEN)[] mm.c#_put_page_type+0x107/0x340 (XEN)[] mm.c#put_page_from_l4e+0x106/0x130 (XEN)[] free_page_type+0x625/0x790 (XEN)[] mm.c#_put_page_type+0x107/0x340 (XEN)[] put_page_type_preemptible+0xf/0x10 (XEN)[] domain.c#relinquish_memory+0xab/0x460 (XEN)[] pirq_guest_eoi+0x27/0x30 (XEN)[] domain_relinquish_resources+0x203/0x290 (XEN)[] domain_kill+0xbd/0x150 (XEN)[] do_domctl+0x7d3/0x1a90 (XEN)[] do_physdev_op_compat+0/0x70 (XEN)[] do_domctl+0/0x1a90 (XEN)[] pv_hypercall+0x1f5/0x430 (XEN)[] lstar_enter+0xa2/0x120 (XEN)[] lstar_enter+0xae/0x120 (XEN)[] lstar_enter+0xa2/0x120 (XEN)[] lstar_enter+0xae/0x120 (XEN)[] lstar_enter+0xa2/0x120 (XEN)[] lstar_enter+0xae/0x120 (XEN)[] lstar_enter+0x10f/0x120 (XEN) (XEN) (XEN) (XEN) Panic on CPU 0: (XEN) Assertion 'cpu < nr_cpu_ids' failed at ...1/work/xen-4.11.0-rc1/xen/include/xen/cpumask.h:97 (XEN) (XEN) (XEN) Reboot in five seconds... (XEN) parameter "gnttab_max_nr_frames" unknown! Xen 4.11-rcnb0 (XEN) Xen version 4.11-rcnb0 (bouyer@) (gcc (nb2 20150115) 4.8.5) debug=y Tue Apr 24 16:10:25 MEST 2018 (XEN) Latest ChangeSet: (XEN) Console output is synchronous. (XEN) Bootloader: unknown (XEN) Command line: dom0_mem=512M console=com1 com1=9
[Xen-devel] [linux-linus test] 122358: regressions - FAIL
flight 122358 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/122358/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl-multivcpu 10 debian-install fail REGR. vs. 118324 test-armhf-armhf-xl-arndale 10 debian-install fail REGR. vs. 118324 test-armhf-armhf-xl-xsm 10 debian-install fail REGR. vs. 118324 test-armhf-armhf-xl-cubietruck 10 debian-install fail REGR. vs. 118324 test-armhf-armhf-libvirt 10 debian-install fail REGR. vs. 118324 Regressions which are regarded as allowable (not blocking): test-armhf-armhf-xl-rtds 10 debian-install fail REGR. vs. 118324 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 118324 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail like 118324 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 118324 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 118324 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 118324 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 118324 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 118324 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 118324 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-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-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-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-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-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-libvirt-raw 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 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-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-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 windows-installfail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass version targeted for testing: linux6d08b06e67cd117f6992c46611dfb4ce267cd71e baseline version: linux5b7d27967dabfb17c21b0d98b29153b9e3ee71e5 Last test of basis 118324 2018-01-25 07:31:24 Z 89 days Failing since118362 2018-01-26 16:56:17 Z 87 days 72 attempts Testing same since 122358 2018-04-23 11:12:31 Z1 days1 attempts 3341 people touched revisions under test, not listing them all jobs: build-amd64-xsm pass build-arm64-xsm pass build-armhf-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386
Re: [Xen-devel] [PATCH v2 05/10] xen/arm: Setup virtual paging for non-boot CPUs on hotplug/resume
On 04/24/2018 03:50 PM, Mirela Simonovic wrote:
Hi Julien,
Hi Mirela,
Thanks for the feedback.
On Mon, Apr 23, 2018 at 1:28 PM, Julien Grall wrote:
Hi Mirela,
On 20/04/18 13:25, Mirela Simonovic wrote:
In existing code the paging for non-boot CPUs is setup only on boot. The
setup is triggered from start_xen() after all CPUs are brought online.
In other words, the initialization of VTCR_EL2 register is done out of the
cpu_up/start_secondary() control flow. However, the cpu_up flow is also
used
to hotplug non-boot CPUs on resume from suspend to RAM state, in which
case
the virtual paging will not be configured.
With this patch the setting of paging is triggered from start_secondary()
function if the current system state is not boot. This way, the paging
for secondary CPUs will be setup in non-boot scenarios as well, while the
setup in boot scenario remains unchanged.
It is assumed here that after the system completed the boot, CPUs that
execute start_secondary() were booted as well when the Xen itself was
booted. According to this assumption non-boot CPUs will always be
compliant
with the VTCR_EL2 value that was selected by Xen on boot.
Currently, these in no mechanism to trigger hotplugging of a CPU. This
will be added with the suspend to RAM support for ARM, where the hotplug
of non-boot CPUs will be triggered via enable_nonboot_cpus() call.
Signed-off-by: Mirela Simonovic
---
CC: Stefano Stabellini
CC: Julien Grall
---
Changes in v2:
-Fix commit message
-Save configured VTCR_EL2 value into static variable that will be used
by non-boot CPUs on hotplug
-Add setup_virt_paging_secondary() and invoke it from start_secondary()
if that CPU has to setup virtual paging (if the system state is not
boot)
---
xen/arch/arm/p2m.c| 13 -
xen/arch/arm/smpboot.c| 3 +++
xen/include/asm-arm/p2m.h | 3 +++
3 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index d43c3aa896..9bb62c13cd 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1451,13 +1451,21 @@ err:
return page;
}
-static void __init setup_virt_paging_one(void *data)
+static void setup_virt_paging_one(void *data)
{
unsigned long val = (unsigned long)data;
WRITE_SYSREG32(val, VTCR_EL2);
isb();
}
+/* VTCR value to be configured by all CPUs. Set only once by the boot
CPU */
+static unsigned long __read_mostly vtcr_value;
VTCR is a register, so the type should be represented in term of bits (i.e
uint*_t).
I followed the type used in setup_virt_paging() and it's unsigned
long. However, the spec says the VTCR_EL2 is 32-bit register, meaning
that in the current implementation the type is not correct.
If I want the type to be correct in this patch Xen will not compile.
Are you suggesting to fix the type in existing implementation?
+
+void setup_virt_paging_secondary(void)
+{
+setup_virt_paging_one((void *)vtcr_value);
That's fairly ugly. Is there any way to rework the interface? For instance,
because you have a static variable which contain the VTCR, you could just
use the variable in setup_virt_paging one.
If the argument provided to setup_virt_paging_one() is NULL within the
setup_virt_paging_one() I configure saved static vtcr_value? If that
is what you meant it was submitted in previous version of this patch
:)
Are you suggesting to revert the change to v1?
+}
+
void __init setup_virt_paging(void)
{
/* Setup Stage 2 address translation */
@@ -1540,6 +1548,9 @@ void __init setup_virt_paging(void)
BUG_ON( P2M_ROOT_LEVEL == 0 && P2M_ROOT_ORDER > 0 );
setup_virt_paging_one((void *)val);
smp_call_function(setup_virt_paging_one, (void *)val, 1);
+
+/* Save configured value (to be used later for secondary CPUs
hotplug) */
+vtcr_value = val;
}
/*
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index 38b665a6d2..abc642804f 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -358,6 +358,9 @@ void start_secondary(unsigned long boot_phys_offset,
local_irq_enable();
local_abort_enable();
+if ( system_state != SYS_STATE_boot )
+setup_virt_paging_secondary();
I think this code needs some documentation. So people understand why you
only call setup_virt_paging_secondary() after boot. But is there any reason
to not use a notifier (see notify_cpu_starting)? This would avoid yet
another export.
It works using notifiers, but I wouldn't say it's worth the overhead.
Implementation using notifiers requires 1 additional data structure
(notifier_block) and 2 functions to be implemented (an __init function
to register a notifier and another one for callback).
Moreover, such a callback would be called for each CPU event, which is
3 times when the CPU is hot-unplugged and nothing needs to be done.
I've already implemented notifier-based approach so I have no problem
submitting it. However, I'm not sure
Re: [Xen-devel] [PATCH v2 3/5] ALSA: xen-front: Implement Xen event channel handling
On 04/24/2018 06:02 PM, Takashi Iwai wrote:
On Tue, 24 Apr 2018 16:58:43 +0200,
Oleksandr Andrushchenko wrote:
On 04/24/2018 05:35 PM, Takashi Iwai wrote:
On Tue, 24 Apr 2018 16:29:15 +0200,
Oleksandr Andrushchenko wrote:
On 04/24/2018 05:20 PM, Takashi Iwai wrote:
On Mon, 16 Apr 2018 08:24:51 +0200,
Oleksandr Andrushchenko wrote:
+static irqreturn_t evtchnl_interrupt_req(int irq, void *dev_id)
+{
+ struct xen_snd_front_evtchnl *channel = dev_id;
+ struct xen_snd_front_info *front_info = channel->front_info;
+ struct xensnd_resp *resp;
+ RING_IDX i, rp;
+ unsigned long flags;
+
+ if (unlikely(channel->state != EVTCHNL_STATE_CONNECTED))
+ return IRQ_HANDLED;
+
+ spin_lock_irqsave(&front_info->io_lock, flags);
+
+again:
+ rp = channel->u.req.ring.sring->rsp_prod;
+ /* ensure we see queued responses up to rp */
+ rmb();
+
+ for (i = channel->u.req.ring.rsp_cons; i != rp; i++) {
I'm not familiar with Xen stuff in general, but through a quick
glance, this kind of code worries me a bit.
If channel->u.req.ring.rsp_cons has a bogus number, this may lead to a
very long loop, no? Better to have a sanity check of the ring buffer
size.
In this loop I have:
resp = RING_GET_RESPONSE(&channel->u.req.ring, i);
and the RING_GET_RESPONSE macro is designed in the way that
it wraps around when *i* in the question gets bigger than
the ring size:
#define RING_GET_REQUEST(_r, _idx) \
(&((_r)->sring->ring[((_idx) & (RING_SIZE(_r) - 1))].req))
So, even if the counter has a bogus number it will not last long
Hm, this prevents from accessing outside the ring buffer, but does it
change the loop behavior?
no, it doesn't
Suppose channel->u.req.ring_rsp_cons = 1, and rp = 0, the loop below
would still consume the whole 32bit counts, no?
for (i = channel->u.req.ring.rsp_cons; i != rp; i++) {
resp = RING_GET_RESPONSE(&channel->u.req.ring, i);
...
}
You are right here and the comment is totally valid.
I'll put an additional check like here [1] and here [2]
Will this address your comment?
Yep, this kind of sanity checks should work.
Great, will implement the checks this way then
thanks,
Takashi
Thank you,
Oleksandr
Takashi
Thank you,
Oleksandr
[1]
https://elixir.bootlin.com/linux/v4.17-rc2/source/drivers/block/xen-blkback/blkback.c#L1127
[2]
https://elixir.bootlin.com/linux/v4.17-rc2/source/drivers/block/xen-blkback/blkback.c#L1135
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 4/6] arm: add a small kconfig for Renesas RCar H3
Hi, On 04/24/2018 03:18 PM, Andrii Anisov wrote: Can you quantify what would be the cost of keeping that code around for IOMMU-less platform? I'm not sure I understand your question. Do you mean a number of loc of the passthrough feature for arm? I meant that disabling something in Xen will come with a cost. While for a driver the maintenance is fairly minimal for anything touching core Xen it will require some more work for any change. So I understand that it will make Xen slightly smaller (~600 lines), but at what cost? In other words, I am all for disabling unnecessary driver in Xen with some caveats (see my other answers). But I am quite worry on the burden for anything else without any real assessment. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH 15/16] os-posix: cleanup: Replace perror with error_report
Philippe Mathieu-Daudé writes:
> On 04/19/2018 01:45 PM, Ian Jackson wrote:
>> perror() is defined to fprintf(stderr,...). HACKING says
>> fprintf(stderr,...) is wrong. So perror() is too.
>>
>> Signed-off-by: Ian Jackson
>> CC: Paolo Bonzini
>> CC: Markus Armbruster
>> CC: Daniel P. Berrange
>> CC: Michael Tokarev
>> CC: Alistair Francis
>> ---
>> v7: New patch
>> ---
>> os-posix.c | 8
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/os-posix.c b/os-posix.c
>> index d4cf466..0108028 100644
>> --- a/os-posix.c
>> +++ b/os-posix.c
>> @@ -125,7 +125,7 @@ void os_set_proc_name(const char *s)
>> /* Could rewrite argv[0] too, but that's a bit more complicated.
>> This simple way is enough for `top'. */
>> if (prctl(PR_SET_NAME, name)) {
>> -perror("unable to change process name");
>> +error_report("unable to change process name: %s", strerror(errno));
>> exit(1);
>> }
>> #else
>> @@ -247,7 +247,7 @@ static void change_root(void)
>> exit(1);
>> }
>> if (chdir("/")) {
>> -perror("not able to chdir to /");
>> +error_report("not able to chdir to /: %s", strerror(errno));
>> exit(1);
>> }
>> }
>> @@ -309,7 +309,7 @@ void os_setup_post(void)
>>
>> if (daemonize) {
>> if (chdir("/")) {
>> -perror("not able to chdir to /");
>> +error_report("not able to chdir to /: %s", strerror(errno));
>> exit(1);
>> }
>> TFR(fd = qemu_open("/dev/null", O_RDWR));
>> @@ -383,7 +383,7 @@ int os_mlock(void)
>>
>> ret = mlockall(MCL_CURRENT | MCL_FUTURE);
>> if (ret < 0) {
>> -perror("mlockall");
>> +error_report("mlockall: %s", strerror(errno));
>> }
>>
>> return ret;
>
> Thinking loudly, maybe we can refactor as error_report_errno(const char
> *desc)...
Maybe. If you want to try, make it a separate series not blocking this
one.
> Anyway:
> Reviewed-by: Philippe Mathieu-Daudé
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/2] Add Brian Woods as Designated reviewer to AMD IOMMU and AMD SVM
On Tue, Apr 24, 2018 at 04:56:23PM +0100, Lars Kurth wrote: > This was discussed in an IRC discussion post the April x86 meeting. > > Signed-off-by: Lars Kurth > --- > MAINTAINERS | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index cc1fdc013f..fab76b0af4 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -144,12 +144,14 @@ F: tools/libacpi/ > > AMD IOMMU > M: Suravee Suthikulpanit > +R: Brian Woods > S: Maintained > F: xen/drivers/passthrough/amd/ > > AMD SVM > M: Boris Ostrovsky > M: Suravee Suthikulpanit > +R: Brian Woods > S: Supported > F: xen/arch/x86/hvm/svm/ > F: xen/arch/x86/cpu/vpmu_amd.c > -- > 2.13.0 > Acked-By: Brian Woods -- Brian Woods ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 02/10] xen/arm: Ignore write to GICD_ISACTIVERn registers (vgic-v2)
Hi Mirela,
On 04/24/2018 12:02 PM, Mirela Simonovic wrote:
Hi Julien,
On Mon, Apr 23, 2018 at 1:15 PM, Julien Grall wrote:
Hi Mirela,
On 20/04/18 13:25, Mirela Simonovic wrote:
Guests attempt to write into these registers on resume (for example
Linux).
Without this patch a data abort exception will be raised to the guest.
This patch handles the write access by ignoring it, but only if the value
to be written is zero. This should be fine because reading these registers
is already handled as 'read as zero'.
Signed-off-by: Mirela Simonovic
---
CC: Stefano Stabellini
CC: Julien Grall
---
Changes in v2:
- Write should be ignored only if the value to be written is zero
(in v1 the write was ignored regardless of the value)
---
xen/arch/arm/vgic-v2.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index 646d1f3d12..afd3e89883 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -488,7 +488,9 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v,
mmio_info_t *info,
printk(XENLOG_G_ERR
"%pv: vGICD: unhandled word write %#"PRIregister" to
ISACTIVER%d\n",
v, r, gicd_reg - GICD_ISACTIVER);
-return 0;
+if ( r != 0 )
+return 0;
It would be better to move the check before the printk. So a warning is
avoided when the guest is writing 0.
If we want to avoid printing a warning when the guest is writing 0
then the printk needs to be moved within the check. I guess this is
what you meant:
if ( r != 0 )
{
printk(XENLOG_G_ERR
"%pv: vGICD: unhandled word write %#"PRIregister" to
ISACTIVER%d\n",
v, r, gicd_reg - GICD_ISACTIVER);
return 0;
}
goto write_ignore_32;
Either that or:
if ( r == 0 )
goto write_ignore_32;
So you don't need to rework the code too much. Both would probably want
some comment in the code.
Please note that in the original patch where I ignored the write
regardless of the value I just followed how it is already done for
GICD_ICACTIVER.
For existing GICD_ICACTIVER case there is no check for the value to be
written and there is a warning printed.
Not checking the value seems fine to me but why is then a warning
printed? Should we suppress that print as well?
The way it is done in ICACTIVER is really fragile. The guest may think
the active bit of the interrupt was cleared but this is not the case.
It is not easy to check if the active bit is set in the current vGIC
(should be better in the new vGIC). So it was decided to just ignore it
to make Linux happy. The warning is here to tell the user that some may
not work as expected.
Regarding the ISACTIVER, you know that if the user write 0 none of the
active state of the interrupts will be changed. So it is fine to avoid
printing the warning. However, if there are one bit set then you really
want to warn the user as the hypervisor will not probably handle it.
So we want to keep the warning in both case.
Cheers,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 2/2] Add Brian Woods as Designated reviewer to AMD IOMMU and AMD SVM
This was discussed in an IRC discussion post the April x86 meeting. Signed-off-by: Lars Kurth --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index cc1fdc013f..fab76b0af4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -144,12 +144,14 @@ F:tools/libacpi/ AMD IOMMU M: Suravee Suthikulpanit +R: Brian Woods S: Maintained F: xen/drivers/passthrough/amd/ AMD SVM M: Boris Ostrovsky M: Suravee Suthikulpanit +R: Brian Woods S: Supported F: xen/arch/x86/hvm/svm/ F: xen/arch/x86/cpu/vpmu_amd.c -- 2.13.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 0/2] Add Designated Reviewer (R:) to MAINTAINERS (plus a test case)
This follows up from a conversation after the April x86 community call, in which I had the following action: Lars to propose fixing CC issue in xen.git:MAINTAINERS copying the R section entries from Linux.git:MAINTAINERS (will need changes to get_maintainers.pl also) Lars Kurth (2): Add Designated Reviewer (R:) to MAINTAINERS file and add support for it in get_maintainer.pl Add Brian Woods as Designated reviewer to AMD IOMMU and AMD SVM MAINTAINERS | 4 scripts/get_maintainer.pl | 24 +++- 2 files changed, 15 insertions(+), 13 deletions(-) -- 2.13.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 1/2] Add Designated Reviewer (R:) to MAINTAINERS file and add support for it in get_maintainer.pl
The syntax has been copied from the Linux Maintainers file. I moved the
following Linux
get_maintainer.pl patches to Xen, fixing up some merge issues (and a bug).
The get_maintainer.pl changes were based on the following git commits
*
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/scripts/get_maintainer.pl?id=
* c1c3f2c906e35bcb6e4cdf5b8e077660fead14fe
* 4f07510df2e8c47fd65b8ffaaf6c5d334d59d598
I also removed code related to
P: Person (obsolete)
which is in the Linux MAINTAINER's file, but not ours. I may not have
caught all instances though.
I have tested on a number of files using mock entries in MAINTAINERS
using ./scripts/get_maintainer.pl -f ...
I also tested --nor to disable the support and it worked as expected.
Signed-off-by: Lars Kurth
---
MAINTAINERS | 2 ++
scripts/get_maintainer.pl | 24 +++-
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index bbda4b9f43..cc1fdc013f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -60,6 +60,8 @@ appropriate branch.
Descriptions of section entries:
M: Mail patches to: FullName
+ R: Designated reviewer: FullName
+ These reviewers should be CCed on patches.
L: Mailing list that is relevant to this area
W: Web-page with status/info
T: SCM tree type and location. Type is one of: git, hg, quilt, stgit.
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 3fb1ad4b69..d528da738c 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -21,6 +21,7 @@ my $xen_path = "./";
my $email = 1;
my $email_usename = 1;
my $email_maintainer = 1;
+my $email_reviewer = 1;
my $email_list = 1;
my $email_subscriber_list = 0;
my $email_git_penguin_chiefs = 0;
@@ -199,6 +200,7 @@ if (!GetOptions(
'mailmap!' => \$email_use_mailmap,
'drop_the_rest_supporter!' =>
\$email_drop_the_rest_supporter_if_supporter_found,
'm!' => \$email_maintainer,
+ 'r!' => \$email_reviewer,
'n!' => \$email_usename,
'l!' => \$email_list,
's!' => \$email_subscriber_list,
@@ -257,7 +259,8 @@ if ($sections) {
}
if ($email &&
-($email_maintainer + $email_list + $email_subscriber_list +
+($email_maintainer + $email_reviewer +
+ $email_list + $email_subscriber_list +
$email_git + $email_git_penguin_chiefs + $email_git_blame) == 0) {
die "$P: Please select at least 1 email option\n";
}
@@ -791,6 +794,7 @@ MAINTAINER field selection options:
--hg-since => hg history to use (default: $email_hg_since)
--interactive => display a menu (mostly useful if used with the --git
option)
--m => include maintainer(s) if any
+--r => include reviewer(s) if any
--n => include name 'Full Name '
--l => include list(s) if any
--s => include subscriber only list(s) if any
@@ -817,7 +821,7 @@ Other options:
--help => show this help information
Default options:
- [--email --nogit --git-fallback --m --n --l --multiline -pattern-depth=0
+ [--email --nogit --git-fallback --m --r --n --l --multiline -pattern-depth=0
--remove-duplicates --rolestats]
Notes:
@@ -1080,21 +1084,15 @@ sub add_categories {
}
} elsif ($ptype eq "M") {
my ($name, $address) = parse_email($pvalue);
- if ($name eq "") {
- if ($i > 0) {
- my $tv = $typevalue[$i - 1];
- if ($tv =~ m/^([A-Z]):\s*(.*)/) {
- if ($1 eq "P") {
- $name = $2;
- $pvalue = format_email($name, $address,
$email_usename);
- }
- }
- }
- }
if ($email_maintainer) {
my $role = get_maintainer_role($i);
push_email_addresses($pvalue, $role);
}
+ } elsif ($ptype eq "R") {
+ my ($name, $address) = parse_email($pvalue);
+ if ($email_reviewer) {
+ push_email_addresses($pvalue, 'reviewer');
+ }
} elsif ($ptype eq "T") {
push(@scm, $pvalue);
} elsif ($ptype eq "W") {
--
2.13.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH 15/16] os-posix: cleanup: Replace perror with error_report
On Tue, Apr 24, 2018 at 10:43:09AM -0500, Eric Blake wrote:
> On 04/24/2018 10:40 AM, Eric Blake wrote:
> > On 04/24/2018 10:18 AM, Daniel P. Berrangé wrote:
> >
> >>> - static void vreport(report_type type, const char *fmt, va_list ap)
> >>> + static void vreport(report_type type, int errnoval, const char *fmt,
> >>> va_list ap)
> >>> ...
> >>> + if (errnoval >= 0) {
> >>> + error_printf(": %s", strerror(errnoval);
> >>> + }
> >>>
> >>> and then add both
> >>> error_report_errno
> >>> error_vreport_errno
> >>> with the obvious semantics.
> >>
> >> That would be nice, because then we can make these two functions actually
> >> use strerror_r() instead of strerror(), for thread safety on all platforms.
> >
> > Except that strerror_r() is a bear to use portably, given that glibc's
> > default declaration differs from the POSIX requirement (you can force
> > glibc to give you the POSIX version, but doing so causes you to lose
> > access to many other useful extensions). It's rather telling that 'git
> > grep strerror_r' currently comes up empty.
>
> That said, glib's g_strerror() may be suitable for this purpose,
> although we are currently using it only in tests/ivshmem-test.c.
Yes, that uses strerror_r internally, or strerror_s on Windows:
https://gitlab.gnome.org/GNOME/glib/blob/master/glib/gstrfuncs.c#L1236
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH 15/16] os-posix: cleanup: Replace perror with error_report
On 04/24/2018 10:40 AM, Eric Blake wrote:
> On 04/24/2018 10:18 AM, Daniel P. Berrangé wrote:
>
>>> - static void vreport(report_type type, const char *fmt, va_list ap)
>>> + static void vreport(report_type type, int errnoval, const char *fmt,
>>> va_list ap)
>>> ...
>>> + if (errnoval >= 0) {
>>> + error_printf(": %s", strerror(errnoval);
>>> + }
>>>
>>> and then add both
>>> error_report_errno
>>> error_vreport_errno
>>> with the obvious semantics.
>>
>> That would be nice, because then we can make these two functions actually
>> use strerror_r() instead of strerror(), for thread safety on all platforms.
>
> Except that strerror_r() is a bear to use portably, given that glibc's
> default declaration differs from the POSIX requirement (you can force
> glibc to give you the POSIX version, but doing so causes you to lose
> access to many other useful extensions). It's rather telling that 'git
> grep strerror_r' currently comes up empty.
That said, glib's g_strerror() may be suitable for this purpose,
although we are currently using it only in tests/ivshmem-test.c.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
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] [Qemu-devel] [PATCH 15/16] os-posix: cleanup: Replace perror with error_report
On 04/24/2018 10:18 AM, Daniel P. Berrangé wrote:
>> - static void vreport(report_type type, const char *fmt, va_list ap)
>> + static void vreport(report_type type, int errnoval, const char *fmt,
>> va_list ap)
>> ...
>> + if (errnoval >= 0) {
>> + error_printf(": %s", strerror(errnoval);
>> + }
>>
>> and then add both
>> error_report_errno
>> error_vreport_errno
>> with the obvious semantics.
>
> That would be nice, because then we can make these two functions actually
> use strerror_r() instead of strerror(), for thread safety on all platforms.
Except that strerror_r() is a bear to use portably, given that glibc's
default declaration differs from the POSIX requirement (you can force
glibc to give you the POSIX version, but doing so causes you to lose
access to many other useful extensions). It's rather telling that 'git
grep strerror_r' currently comes up empty.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
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] [Qemu-devel] [PATCH 15/16] os-posix: cleanup: Replace perror with error_report
On Tue, Apr 24, 2018 at 03:53:48PM +0100, Ian Jackson wrote:
> Philippe Mathieu-Daudé writes ("Re: [Qemu-devel] [PATCH 15/16] os-posix:
> cleanup: Replace perror with error_report"):
> > On 04/19/2018 01:45 PM, Ian Jackson wrote:
> > > -perror("mlockall");
> > > +error_report("mlockall: %s", strerror(errno));
> > > }
> > >
> > > return ret;
> >
> > Thinking loudly, maybe we can refactor as error_report_errno(const char
> > *desc)...
>
> git-grep 'error_report.*errno' shows a lot of call sites that do
> something more exciting than const char *desc would support.
>
> I think the right approach would be
>
> - static void vreport(report_type type, const char *fmt, va_list ap)
> + static void vreport(report_type type, int errnoval, const char *fmt,
> va_list ap)
> ...
> + if (errnoval >= 0) {
> + error_printf(": %s", strerror(errnoval);
> + }
>
> and then add both
> error_report_errno
> error_vreport_errno
> with the obvious semantics.
That would be nice, because then we can make these two functions actually
use strerror_r() instead of strerror(), for thread safety on all platforms.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 05/16] xen: defer call to xen_restrict until just before os_setup_post
Anthony PERARD writes ("Re: [PATCH 05/16] xen: defer call to xen_restrict until
just before os_setup_post"):
> I think this include is not needed anymore, and can go away from the
> patch series.
Yes.
Thanks,
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH 16/16] configure: do_compiler: Dump some extra info under bash
Eric Blake writes ("Re: [Qemu-devel] [PATCH 16/16] configure: do_compiler: Dump
some extra info under bash"):
> That's still fork-heavy. You could do:
>
> test -n "$BASH_VERSION" && eval '
> echo >>config.log "
> funcs: ${FUNCNAME[*]}
> lines: ${BASH_LINENO[*]}
> files: ${BASH_SOURCE[*]}"'
>
> which avoids the fork, but remains silent on dash.
Thanks. I will adopt this. Although I will use if ... then as that
seems to be the usual style. (&& would break with set -e which
configure does not use...)
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 3/5] ALSA: xen-front: Implement Xen event channel handling
On Tue, 24 Apr 2018 16:58:43 +0200,
Oleksandr Andrushchenko wrote:
>
> On 04/24/2018 05:35 PM, Takashi Iwai wrote:
> > On Tue, 24 Apr 2018 16:29:15 +0200,
> > Oleksandr Andrushchenko wrote:
> >> On 04/24/2018 05:20 PM, Takashi Iwai wrote:
> >>> On Mon, 16 Apr 2018 08:24:51 +0200,
> >>> Oleksandr Andrushchenko wrote:
> +static irqreturn_t evtchnl_interrupt_req(int irq, void *dev_id)
> +{
> +struct xen_snd_front_evtchnl *channel = dev_id;
> +struct xen_snd_front_info *front_info = channel->front_info;
> +struct xensnd_resp *resp;
> +RING_IDX i, rp;
> +unsigned long flags;
> +
> +if (unlikely(channel->state != EVTCHNL_STATE_CONNECTED))
> +return IRQ_HANDLED;
> +
> +spin_lock_irqsave(&front_info->io_lock, flags);
> +
> +again:
> +rp = channel->u.req.ring.sring->rsp_prod;
> +/* ensure we see queued responses up to rp */
> +rmb();
> +
> +for (i = channel->u.req.ring.rsp_cons; i != rp; i++) {
> >>> I'm not familiar with Xen stuff in general, but through a quick
> >>> glance, this kind of code worries me a bit.
> >>>
> >>> If channel->u.req.ring.rsp_cons has a bogus number, this may lead to a
> >>> very long loop, no? Better to have a sanity check of the ring buffer
> >>> size.
> >> In this loop I have:
> >> resp = RING_GET_RESPONSE(&channel->u.req.ring, i);
> >> and the RING_GET_RESPONSE macro is designed in the way that
> >> it wraps around when *i* in the question gets bigger than
> >> the ring size:
> >>
> >> #define RING_GET_REQUEST(_r, _idx) \
> >> (&((_r)->sring->ring[((_idx) & (RING_SIZE(_r) - 1))].req))
> >>
> >> So, even if the counter has a bogus number it will not last long
> > Hm, this prevents from accessing outside the ring buffer, but does it
> > change the loop behavior?
> no, it doesn't
> > Suppose channel->u.req.ring_rsp_cons = 1, and rp = 0, the loop below
> > would still consume the whole 32bit counts, no?
> >
> > for (i = channel->u.req.ring.rsp_cons; i != rp; i++) {
> > resp = RING_GET_RESPONSE(&channel->u.req.ring, i);
> > ...
> > }
> You are right here and the comment is totally valid.
> I'll put an additional check like here [1] and here [2]
> Will this address your comment?
Yep, this kind of sanity checks should work.
thanks,
Takashi
> >
> > Takashi
> Thank you,
> Oleksandr
>
> [1]
> https://elixir.bootlin.com/linux/v4.17-rc2/source/drivers/block/xen-blkback/blkback.c#L1127
> [2]
> https://elixir.bootlin.com/linux/v4.17-rc2/source/drivers/block/xen-blkback/blkback.c#L1135
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 3/5] ALSA: xen-front: Implement Xen event channel handling
On 04/24/2018 05:35 PM, Takashi Iwai wrote:
On Tue, 24 Apr 2018 16:29:15 +0200,
Oleksandr Andrushchenko wrote:
On 04/24/2018 05:20 PM, Takashi Iwai wrote:
On Mon, 16 Apr 2018 08:24:51 +0200,
Oleksandr Andrushchenko wrote:
+static irqreturn_t evtchnl_interrupt_req(int irq, void *dev_id)
+{
+ struct xen_snd_front_evtchnl *channel = dev_id;
+ struct xen_snd_front_info *front_info = channel->front_info;
+ struct xensnd_resp *resp;
+ RING_IDX i, rp;
+ unsigned long flags;
+
+ if (unlikely(channel->state != EVTCHNL_STATE_CONNECTED))
+ return IRQ_HANDLED;
+
+ spin_lock_irqsave(&front_info->io_lock, flags);
+
+again:
+ rp = channel->u.req.ring.sring->rsp_prod;
+ /* ensure we see queued responses up to rp */
+ rmb();
+
+ for (i = channel->u.req.ring.rsp_cons; i != rp; i++) {
I'm not familiar with Xen stuff in general, but through a quick
glance, this kind of code worries me a bit.
If channel->u.req.ring.rsp_cons has a bogus number, this may lead to a
very long loop, no? Better to have a sanity check of the ring buffer
size.
In this loop I have:
resp = RING_GET_RESPONSE(&channel->u.req.ring, i);
and the RING_GET_RESPONSE macro is designed in the way that
it wraps around when *i* in the question gets bigger than
the ring size:
#define RING_GET_REQUEST(_r, _idx) \
(&((_r)->sring->ring[((_idx) & (RING_SIZE(_r) - 1))].req))
So, even if the counter has a bogus number it will not last long
Hm, this prevents from accessing outside the ring buffer, but does it
change the loop behavior?
no, it doesn't
Suppose channel->u.req.ring_rsp_cons = 1, and rp = 0, the loop below
would still consume the whole 32bit counts, no?
for (i = channel->u.req.ring.rsp_cons; i != rp; i++) {
resp = RING_GET_RESPONSE(&channel->u.req.ring, i);
...
}
You are right here and the comment is totally valid.
I'll put an additional check like here [1] and here [2]
Will this address your comment?
Takashi
Thank you,
Oleksandr
[1]
https://elixir.bootlin.com/linux/v4.17-rc2/source/drivers/block/xen-blkback/blkback.c#L1127
[2]
https://elixir.bootlin.com/linux/v4.17-rc2/source/drivers/block/xen-blkback/blkback.c#L1135
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH 15/16] os-posix: cleanup: Replace perror with error_report
Philippe Mathieu-Daudé writes ("Re: [Qemu-devel] [PATCH 15/16] os-posix:
cleanup: Replace perror with error_report"):
> On 04/19/2018 01:45 PM, Ian Jackson wrote:
> > -perror("mlockall");
> > +error_report("mlockall: %s", strerror(errno));
> > }
> >
> > return ret;
>
> Thinking loudly, maybe we can refactor as error_report_errno(const char
> *desc)...
git-grep 'error_report.*errno' shows a lot of call sites that do
something more exciting than const char *desc would support.
I think the right approach would be
- static void vreport(report_type type, const char *fmt, va_list ap)
+ static void vreport(report_type type, int errnoval, const char *fmt, va_list
ap)
...
+ if (errnoval >= 0) {
+ error_printf(": %s", strerror(errnoval);
+ }
and then add both
error_report_errno
error_vreport_errno
with the obvious semantics.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 05/10] xen/arm: Setup virtual paging for non-boot CPUs on hotplug/resume
Hi Julien,
Thanks for the feedback.
On Mon, Apr 23, 2018 at 1:28 PM, Julien Grall wrote:
> Hi Mirela,
>
>
> On 20/04/18 13:25, Mirela Simonovic wrote:
>>
>> In existing code the paging for non-boot CPUs is setup only on boot. The
>> setup is triggered from start_xen() after all CPUs are brought online.
>> In other words, the initialization of VTCR_EL2 register is done out of the
>> cpu_up/start_secondary() control flow. However, the cpu_up flow is also
>> used
>> to hotplug non-boot CPUs on resume from suspend to RAM state, in which
>> case
>> the virtual paging will not be configured.
>> With this patch the setting of paging is triggered from start_secondary()
>> function if the current system state is not boot. This way, the paging
>> for secondary CPUs will be setup in non-boot scenarios as well, while the
>> setup in boot scenario remains unchanged.
>> It is assumed here that after the system completed the boot, CPUs that
>> execute start_secondary() were booted as well when the Xen itself was
>> booted. According to this assumption non-boot CPUs will always be
>> compliant
>> with the VTCR_EL2 value that was selected by Xen on boot.
>> Currently, these in no mechanism to trigger hotplugging of a CPU. This
>> will be added with the suspend to RAM support for ARM, where the hotplug
>> of non-boot CPUs will be triggered via enable_nonboot_cpus() call.
>>
>> Signed-off-by: Mirela Simonovic
>>
>> ---
>> CC: Stefano Stabellini
>> CC: Julien Grall
>> ---
>> Changes in v2:
>> -Fix commit message
>> -Save configured VTCR_EL2 value into static variable that will be used
>> by non-boot CPUs on hotplug
>> -Add setup_virt_paging_secondary() and invoke it from start_secondary()
>> if that CPU has to setup virtual paging (if the system state is not
>> boot)
>> ---
>> xen/arch/arm/p2m.c| 13 -
>> xen/arch/arm/smpboot.c| 3 +++
>> xen/include/asm-arm/p2m.h | 3 +++
>> 3 files changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
>> index d43c3aa896..9bb62c13cd 100644
>> --- a/xen/arch/arm/p2m.c
>> +++ b/xen/arch/arm/p2m.c
>> @@ -1451,13 +1451,21 @@ err:
>> return page;
>> }
>> -static void __init setup_virt_paging_one(void *data)
>> +static void setup_virt_paging_one(void *data)
>> {
>> unsigned long val = (unsigned long)data;
>> WRITE_SYSREG32(val, VTCR_EL2);
>> isb();
>> }
>> +/* VTCR value to be configured by all CPUs. Set only once by the boot
>> CPU */
>> +static unsigned long __read_mostly vtcr_value;
>
>
> VTCR is a register, so the type should be represented in term of bits (i.e
> uint*_t).
I followed the type used in setup_virt_paging() and it's unsigned
long. However, the spec says the VTCR_EL2 is 32-bit register, meaning
that in the current implementation the type is not correct.
If I want the type to be correct in this patch Xen will not compile.
Are you suggesting to fix the type in existing implementation?
>
>> +
>> +void setup_virt_paging_secondary(void)
>> +{
>> +setup_virt_paging_one((void *)vtcr_value);
>
>
> That's fairly ugly. Is there any way to rework the interface? For instance,
> because you have a static variable which contain the VTCR, you could just
> use the variable in setup_virt_paging one.
>
If the argument provided to setup_virt_paging_one() is NULL within the
setup_virt_paging_one() I configure saved static vtcr_value? If that
is what you meant it was submitted in previous version of this patch
:)
Are you suggesting to revert the change to v1?
>> +}
>> +
>> void __init setup_virt_paging(void)
>> {
>> /* Setup Stage 2 address translation */
>> @@ -1540,6 +1548,9 @@ void __init setup_virt_paging(void)
>> BUG_ON( P2M_ROOT_LEVEL == 0 && P2M_ROOT_ORDER > 0 );
>> setup_virt_paging_one((void *)val);
>> smp_call_function(setup_virt_paging_one, (void *)val, 1);
>> +
>> +/* Save configured value (to be used later for secondary CPUs
>> hotplug) */
>> +vtcr_value = val;
>> }
>> /*
>> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
>> index 38b665a6d2..abc642804f 100644
>> --- a/xen/arch/arm/smpboot.c
>> +++ b/xen/arch/arm/smpboot.c
>> @@ -358,6 +358,9 @@ void start_secondary(unsigned long boot_phys_offset,
>> local_irq_enable();
>> local_abort_enable();
>> +if ( system_state != SYS_STATE_boot )
>> +setup_virt_paging_secondary();
>
> I think this code needs some documentation. So people understand why you
> only call setup_virt_paging_secondary() after boot. But is there any reason
> to not use a notifier (see notify_cpu_starting)? This would avoid yet
> another export.
It works using notifiers, but I wouldn't say it's worth the overhead.
Implementation using notifiers requires 1 additional data structure
(notifier_block) and 2 functions to be implemented (an __init function
to register a notifier and another one for callback).
Moreover, such a callback would be called for each CPU event, wh
[Xen-devel] [xen-unstable-smoke test] 122392: tolerable all pass - PUSHED
flight 122392 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/122392/ 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 27170adb54a558e11defcd51989326a9beb95afe baseline version: xen d80af845de7a4db01a4a3b4d779e0e0dcb5e738b Last test of basis 122371 2018-04-23 15:02:22 Z0 days Testing same since 122392 2018-04-24 13:00:34 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Jan Beulich jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-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 d80af845de..27170adb54 27170adb54a558e11defcd51989326a9beb95afe -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 3/5] ALSA: xen-front: Implement Xen event channel handling
On Tue, 24 Apr 2018 16:29:15 +0200,
Oleksandr Andrushchenko wrote:
>
> On 04/24/2018 05:20 PM, Takashi Iwai wrote:
> > On Mon, 16 Apr 2018 08:24:51 +0200,
> > Oleksandr Andrushchenko wrote:
> >> +static irqreturn_t evtchnl_interrupt_req(int irq, void *dev_id)
> >> +{
> >> + struct xen_snd_front_evtchnl *channel = dev_id;
> >> + struct xen_snd_front_info *front_info = channel->front_info;
> >> + struct xensnd_resp *resp;
> >> + RING_IDX i, rp;
> >> + unsigned long flags;
> >> +
> >> + if (unlikely(channel->state != EVTCHNL_STATE_CONNECTED))
> >> + return IRQ_HANDLED;
> >> +
> >> + spin_lock_irqsave(&front_info->io_lock, flags);
> >> +
> >> +again:
> >> + rp = channel->u.req.ring.sring->rsp_prod;
> >> + /* ensure we see queued responses up to rp */
> >> + rmb();
> >> +
> >> + for (i = channel->u.req.ring.rsp_cons; i != rp; i++) {
> > I'm not familiar with Xen stuff in general, but through a quick
> > glance, this kind of code worries me a bit.
> >
> > If channel->u.req.ring.rsp_cons has a bogus number, this may lead to a
> > very long loop, no? Better to have a sanity check of the ring buffer
> > size.
> In this loop I have:
> resp = RING_GET_RESPONSE(&channel->u.req.ring, i);
> and the RING_GET_RESPONSE macro is designed in the way that
> it wraps around when *i* in the question gets bigger than
> the ring size:
>
> #define RING_GET_REQUEST(_r, _idx) \
> (&((_r)->sring->ring[((_idx) & (RING_SIZE(_r) - 1))].req))
>
> So, even if the counter has a bogus number it will not last long
Hm, this prevents from accessing outside the ring buffer, but does it
change the loop behavior?
Suppose channel->u.req.ring_rsp_cons = 1, and rp = 0, the loop below
would still consume the whole 32bit counts, no?
for (i = channel->u.req.ring.rsp_cons; i != rp; i++) {
resp = RING_GET_RESPONSE(&channel->u.req.ring, i);
...
}
Takashi
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 3/5] ALSA: xen-front: Implement Xen event channel handling
On 04/24/2018 05:20 PM, Takashi Iwai wrote:
On Mon, 16 Apr 2018 08:24:51 +0200,
Oleksandr Andrushchenko wrote:
+static irqreturn_t evtchnl_interrupt_req(int irq, void *dev_id)
+{
+ struct xen_snd_front_evtchnl *channel = dev_id;
+ struct xen_snd_front_info *front_info = channel->front_info;
+ struct xensnd_resp *resp;
+ RING_IDX i, rp;
+ unsigned long flags;
+
+ if (unlikely(channel->state != EVTCHNL_STATE_CONNECTED))
+ return IRQ_HANDLED;
+
+ spin_lock_irqsave(&front_info->io_lock, flags);
+
+again:
+ rp = channel->u.req.ring.sring->rsp_prod;
+ /* ensure we see queued responses up to rp */
+ rmb();
+
+ for (i = channel->u.req.ring.rsp_cons; i != rp; i++) {
I'm not familiar with Xen stuff in general, but through a quick
glance, this kind of code worries me a bit.
If channel->u.req.ring.rsp_cons has a bogus number, this may lead to a
very long loop, no? Better to have a sanity check of the ring buffer
size.
In this loop I have:
resp = RING_GET_RESPONSE(&channel->u.req.ring, i);
and the RING_GET_RESPONSE macro is designed in the way that
it wraps around when *i* in the question gets bigger than
the ring size:
#define RING_GET_REQUEST(_r, _idx) \
(&((_r)->sring->ring[((_idx) & (RING_SIZE(_r) - 1))].req))
So, even if the counter has a bogus number it will not last long
+static irqreturn_t evtchnl_interrupt_evt(int irq, void *dev_id)
+{
+ struct xen_snd_front_evtchnl *channel = dev_id;
+ struct xen_snd_front_info *front_info = channel->front_info;
+ struct xensnd_event_page *page = channel->u.evt.page;
+ u32 cons, prod;
+ unsigned long flags;
+
+ if (unlikely(channel->state != EVTCHNL_STATE_CONNECTED))
+ return IRQ_HANDLED;
+
+ spin_lock_irqsave(&front_info->io_lock, flags);
+
+ prod = page->in_prod;
+ /* ensure we see ring contents up to prod */
+ virt_rmb();
+ if (prod == page->in_cons)
+ goto out;
+
+ for (cons = page->in_cons; cons != prod; cons++) {
Ditto.
Same as above
thanks,
Takashi
Thank you,
Oleksandr
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 03/16] xen: link against xentoolcore
Anthony PERARD writes ("Re: [PATCH 03/16] xen: link against xentoolcore"):
> On Thu, Apr 19, 2018 at 05:45:06PM +0100, Ian Jackson wrote:
> > xen_pc="xencontrol xenstore xenguest xenforeignmemory xengnttab"
> > -xen_pc="$xen_pc xenevtchn xendevicemodel"
> > +xen_pc="$xen_pc xenevtchn xendevicemodel xentoolcore"
>
> I think we want to check if "xentoolcore" pkg_config exist before trying
> to use it. Otherwith, that is not going to work with Xen older than
> 4.10.
Yes. Thanks for spotting this. My tests were all with builds from in
the Xen tree and I hadn't spotted that that is handled as a special
case.
> Instead, we could do this:
> if $pkg_config --exists xentoolcore; then
> xen_pc+=" xentoolcore"
> fi
I will test that.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 3/5] ALSA: xen-front: Implement Xen event channel handling
On Mon, 16 Apr 2018 08:24:51 +0200,
Oleksandr Andrushchenko wrote:
> +static irqreturn_t evtchnl_interrupt_req(int irq, void *dev_id)
> +{
> + struct xen_snd_front_evtchnl *channel = dev_id;
> + struct xen_snd_front_info *front_info = channel->front_info;
> + struct xensnd_resp *resp;
> + RING_IDX i, rp;
> + unsigned long flags;
> +
> + if (unlikely(channel->state != EVTCHNL_STATE_CONNECTED))
> + return IRQ_HANDLED;
> +
> + spin_lock_irqsave(&front_info->io_lock, flags);
> +
> +again:
> + rp = channel->u.req.ring.sring->rsp_prod;
> + /* ensure we see queued responses up to rp */
> + rmb();
> +
> + for (i = channel->u.req.ring.rsp_cons; i != rp; i++) {
I'm not familiar with Xen stuff in general, but through a quick
glance, this kind of code worries me a bit.
If channel->u.req.ring.rsp_cons has a bogus number, this may lead to a
very long loop, no? Better to have a sanity check of the ring buffer
size.
> +static irqreturn_t evtchnl_interrupt_evt(int irq, void *dev_id)
> +{
> + struct xen_snd_front_evtchnl *channel = dev_id;
> + struct xen_snd_front_info *front_info = channel->front_info;
> + struct xensnd_event_page *page = channel->u.evt.page;
> + u32 cons, prod;
> + unsigned long flags;
> +
> + if (unlikely(channel->state != EVTCHNL_STATE_CONNECTED))
> + return IRQ_HANDLED;
> +
> + spin_lock_irqsave(&front_info->io_lock, flags);
> +
> + prod = page->in_prod;
> + /* ensure we see ring contents up to prod */
> + virt_rmb();
> + if (prod == page->in_cons)
> + goto out;
> +
> + for (cons = page->in_cons; cons != prod; cons++) {
Ditto.
thanks,
Takashi
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 4/6] arm: add a small kconfig for Renesas RCar H3
Can you quantify what would be the cost of keeping that code around for IOMMU-less platform? I'm not sure I understand your question. Do you mean a number of loc of the passthrough feature for arm? -- *Andrii Anisov* ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 1/5] ALSA: xen-front: Introduce Xen para-virtualized sound frontend driver
On 04/24/2018 04:55 PM, Takashi Iwai wrote: On Mon, 16 Apr 2018 08:24:49 +0200, Oleksandr Andrushchenko wrote: --- /dev/null +++ b/sound/xen/Kconfig @@ -0,0 +1,10 @@ +# ALSA Xen drivers + +config SND_XEN_FRONTEND + tristate "Xen para-virtualized sound frontend driver" + depends on XEN && SND_PCM Please do select SND_PCM instead of depends. will do + select XEN_XENBUS_FRONTEND + default n "default n" is superfluous, drop it. will drop thanks, Takashi Thank you, Oleksandr ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 1/5] ALSA: xen-front: Introduce Xen para-virtualized sound frontend driver
On Mon, 16 Apr 2018 08:24:49 +0200, Oleksandr Andrushchenko wrote: > --- /dev/null > +++ b/sound/xen/Kconfig > @@ -0,0 +1,10 @@ > +# ALSA Xen drivers > + > +config SND_XEN_FRONTEND > + tristate "Xen para-virtualized sound frontend driver" > + depends on XEN && SND_PCM Please do select SND_PCM instead of depends. > + select XEN_XENBUS_FRONTEND > + default n "default n" is superfluous, drop it. thanks, Takashi ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.11] x86/spec_ctrl: Fix typo in ARCH_CAPS decode
On 24/04/18 14:35, Konrad Rzeszutek Wilk wrote: > On April 24, 2018 5:44:33 AM EDT, Andrew Cooper > wrote: >> Signed-off-by: Andrew Cooper >> --- >> CC: Jan Beulich >> CC: Juergen Gross > > You are missing an Reported-by.. > > Also pls > Reviewed-by: Konrad Rzeszutek Wilk I remembered and fixed up on commit http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=27170adb54a558e11defcd51989326a9beb95afe ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.11] x86/spec_ctrl: Fix typo in ARCH_CAPS decode
On April 24, 2018 5:44:33 AM EDT, Andrew Cooper wrote: >Signed-off-by: Andrew Cooper >--- >CC: Jan Beulich >CC: Juergen Gross You are missing an Reported-by.. Also pls Reviewed-by: Konrad Rzeszutek Wilk Thx >--- > xen/arch/x86/spec_ctrl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c >index bab8595..fab3c1d 100644 >--- a/xen/arch/x86/spec_ctrl.c >+++ b/xen/arch/x86/spec_ctrl.c >@@ -119,7 +119,7 @@ static void __init print_details(enum ind_thunk >thunk) > (e8b & cpufeat_mask(X86_FEATURE_IBPB)) ? " IBPB" : "", > (caps & ARCH_CAPABILITIES_IBRS_ALL) ? " IBRS_ALL" : "", > (caps & ARCH_CAPABILITIES_RDCL_NO) ? " RDCL_NO" : "", >- (caps & ARCH_CAPS_RSBA) ? " RBSA" : >""); >+ (caps & ARCH_CAPS_RSBA) ? " RSBA" : >""); > > /* Compiled-in support which pertains to BTI mitigations. */ > if ( IS_ENABLED(CONFIG_INDIRECT_THUNK) ) >-- >2.1.4 > > >___ >Xen-devel mailing list >Xen-devel@lists.xenproject.org >https://lists.xenproject.org/mailman/listinfo/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC PATCH 1/1] xen: credit2: rb-tree for runqueues
On Tuesday 24 April 2018 04:33 PM, Dario Faggioli wrote: On Tue, 2018-04-24 at 14:30 +0530, Praveen Kumar wrote: Hi Dario, Hi! On Tuesday 17 April 2018 04:16 PM, Dario Faggioli wrote: On Tue, 2018-04-03 at 22:25 +0530, Praveen Kumar wrote: +if ( svc->credit < entry->credit ) +node = &parent->rb_left; +else +node = &parent->rb_right; + +(*pos)++; +} +rb_link_node(&svc->runq_elem, parent, node); +rb_insert_color(&svc->runq_elem, root); +} Wait, where's the part where we cache which element is the one with the highest credits? (And the same applies to the tree-removal function, of course.) In fact, we need a field for storing such a cache in the runqueue data structure as well, and we need to keep it updated. I thought of caching the left most node as done in rb_tree, but I thought of taking an easy way to have things working and delaying the Linux rb_tree caching variant to be ported in next patch or so. That is fine, as soon as the fact that you are not doing it right now, but planning to do it in another patch is stated somewhere (e.g., cover letter or a changelog). I would suggest we do not try to use the rb_*_cached() functions, and cache rightmost explicitly in runqueue_data. Ok, that sounds better, I will introduce an entry for rightmost element to be cached in runqueue_data Also, lets port the Linux rb_tree cache variant as well ( probably in future we may use that ). I'm not sure about this last part. I mean, I can see other uses of rb- trees, TBH, and ones where such caching would be useful. Still, I'll do the port when we actually decide to use the new functionallity (or when, e.g., we run into issues retro-fitting a Linux fix, etc). Sure, I am fine with that too. Err... is it? Isn't the leftmost element the one with the _least_ credits? It looks to me that we want rb_last(). And IAC, we don't want to have to traverse the tree to get the runnable vcpu with the highest credit, we want it available in O(1) time. That's why we want to cache it. Yes, it looks like an error. Will update the patch in v2. Right. So, I think the main problem with this patch was this one, i.e., the fact that the runqueue was sorted in the wrong order. Then there is the lack of caching, for O(1) access to the head of the runqueue itself. As said, it is ok for that to come in its own patch of this series. It is also ok if this comes as a later patch, as soon as that is clearly stated. Ok. Working on the same. Sure, let me have 3 series, first; Linux porting , second; rb_tree changes which doesn't have caching and third; have rightmost node cached. I'd actually skip doing the porting right now... Although, in this case, it's not really my call, and others may have different a opinion. Sure, I am fine with not including the porting work as part of this patch. Probably, we may take this as a different activity as you mentioned above. Regards, ~Praveen. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen-netfront: fix xennet_start_xmit()'s return type
On Tue, Apr 24, 2018 at 03:18:14PM +0200, Luc Van Oostenryck wrote: > The method ndo_start_xmit() is defined as returning an 'netdev_tx_t', > which is a typedef for an enum type, but the implementation in this > driver returns an 'int'. > > Fix this by returning 'netdev_tx_t' in this driver too. > > Signed-off-by: Luc Van Oostenryck Reviewed-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen-netback: fix xenvif_start_xmit()'s return type
On Tue, Apr 24, 2018 at 03:18:12PM +0200, Luc Van Oostenryck wrote: > The method ndo_start_xmit() is defined as returning an 'netdev_tx_t', > which is a typedef for an enum type, but the implementation in this > driver returns an 'int'. > > Fix this by returning 'netdev_tx_t' in this driver too. > > Signed-off-by: Luc Van Oostenryck Acked-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] xen-netback: fix xenvif_start_xmit()'s return type
The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
which is a typedef for an enum type, but the implementation in this
driver returns an 'int'.
Fix this by returning 'netdev_tx_t' in this driver too.
Signed-off-by: Luc Van Oostenryck
---
drivers/net/xen-netback/interface.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/xen-netback/interface.c
b/drivers/net/xen-netback/interface.c
index 78ebe494f..bb944aa09 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -165,7 +165,7 @@ static u16 xenvif_select_queue(struct net_device *dev,
struct sk_buff *skb,
return vif->hash.mapping[skb_get_hash_raw(skb) % size];
}
-static int xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev)
+static netdev_tx_t xenvif_start_xmit(struct sk_buff *skb, struct net_device
*dev)
{
struct xenvif *vif = netdev_priv(dev);
struct xenvif_queue *queue = NULL;
--
2.17.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] xen-netfront: fix xennet_start_xmit()'s return type
The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
which is a typedef for an enum type, but the implementation in this
driver returns an 'int'.
Fix this by returning 'netdev_tx_t' in this driver too.
Signed-off-by: Luc Van Oostenryck
---
drivers/net/xen-netfront.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 4dd066800..679da1abd 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -564,7 +564,7 @@ static u16 xennet_select_queue(struct net_device *dev,
struct sk_buff *skb,
#define MAX_XEN_SKB_FRAGS (65536 / XEN_PAGE_SIZE + 1)
-static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev)
+static netdev_tx_t xennet_start_xmit(struct sk_buff *skb, struct net_device
*dev)
{
struct netfront_info *np = netdev_priv(dev);
struct netfront_stats *tx_stats = this_cpu_ptr(np->tx_stats);
--
2.17.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [OSSTEST PATCH] cr-for-branches: Add linux-4.14
Signed-off-by: Ian Jackson
---
cr-for-branches | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cr-for-branches b/cr-for-branches
index cf48390..c4de52f 100755
--- a/cr-for-branches
+++ b/cr-for-branches
@@ -31,7 +31,7 @@ scriptoptions="$1"; shift
LOGFILE=tmp/cr-for-branches.log
export LOGFILE
-: ${BRANCHES:=osstest xen-4.0-testing xen-4.1-testing xen-4.2-testing
xen-4.3-testing xen-4.4-testing xen-4.5-testing xen-4.6-testing xen-4.7-testing
xen-4.8-testing xen-4.9-testing xen-4.10-testing xen-unstable qemu-mainline
qemu-upstream-unstable qemu-upstream-4.2-testing qemu-upstream-4.3-testing
qemu-upstream-4.4-testing qemu-upstream-4.5-testing qemu-upstream-4.6-testing
qemu-upstream-4.7-testing qemu-upstream-4.8-testing qemu-upstream-4.9-testing
qemu-upstream-4.10-testing linux-linus linux-4.9 linux-4.1 linux-3.18
linux-3.16 linux-3.14 linux-3.10 linux-3.4 linux-arm-xen seabios ovmf xtf
${EXTRA_BRANCHES}}
+: ${BRANCHES:=osstest xen-4.0-testing xen-4.1-testing xen-4.2-testing
xen-4.3-testing xen-4.4-testing xen-4.5-testing xen-4.6-testing xen-4.7-testing
xen-4.8-testing xen-4.9-testing xen-4.10-testing xen-unstable qemu-mainline
qemu-upstream-unstable qemu-upstream-4.2-testing qemu-upstream-4.3-testing
qemu-upstream-4.4-testing qemu-upstream-4.5-testing qemu-upstream-4.6-testing
qemu-upstream-4.7-testing qemu-upstream-4.8-testing qemu-upstream-4.9-testing
qemu-upstream-4.10-testing linux-linus linux-4.14 linux-4.9 linux-4.1
linux-3.18 linux-3.16 linux-3.14 linux-3.10 linux-3.4 linux-arm-xen seabios
ovmf xtf ${EXTRA_BRANCHES}}
export BRANCHES
fetchwlem=$wlem
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/1] drm/xen-zcopy: Add Xen zero-copy helper DRM driver
On 04/24/2018 02:54 PM, Daniel Vetter wrote: On Mon, Apr 23, 2018 at 03:10:35PM +0300, Oleksandr Andrushchenko wrote: On 04/23/2018 02:52 PM, Wei Liu wrote: On Fri, Apr 20, 2018 at 02:25:20PM +0300, Oleksandr Andrushchenko wrote: the gntdev. I think this is generic enough that it could be implemented by a device not tied to Xen. AFAICT the hyper_dma guys also wanted something similar to this. You can't just wrap random userspace memory into a dma-buf. We've just had this discussion with kvm/qemu folks, who proposed just that, and after a bit of discussion they'll now try to have a driver which just wraps a memfd into a dma-buf. So, we have to decide either we introduce a new driver (say, under drivers/xen/xen-dma-buf) or extend the existing gntdev/balloon to support dma-buf use-cases. Can anybody from Xen community express their preference here? Oleksandr talked to me on IRC about this, he said a few IOCTLs need to be added to either existing drivers or a new driver. I went through this thread twice and skimmed through the relevant documents, but I couldn't see any obvious pros and cons for either approach. So I don't really have an opinion on this. But, assuming if implemented in existing drivers, those IOCTLs need to be added to different drivers, which means userspace program needs to write more code and get more handles, it would be slightly better to implement a new driver from that perspective. If gntdev/balloon extension is still considered: All the IOCTLs will be in gntdev driver (in current xen-zcopy terminology): I was lazy to change dumb to dma-buf, so put this notice ;) - DRM_ICOTL_XEN_ZCOPY_DUMB_FROM_REFS - DRM_IOCTL_XEN_ZCOPY_DUMB_TO_REFS - DRM_IOCTL_XEN_ZCOPY_DUMB_WAIT_FREE s/DUMB/DMA_BUF/ please. This is generic dma-buf, it has nothing to do with the dumb scanout buffer support in the drm/gfx subsystem. This here can be used for any zcopy sharing among guests (as long as your endpoints understands dma-buf, which most relevant drivers do). Of course, please see above -Daniel Balloon driver extension, which is needed for contiguous/DMA buffers, will be to provide new *kernel API*, no UAPI is needed. Wei. Thank you, Oleksandr ___ dri-devel mailing list dri-de...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/1] drm/xen-zcopy: Add Xen zero-copy helper DRM driver
On Mon, Apr 23, 2018 at 03:10:35PM +0300, Oleksandr Andrushchenko wrote: > On 04/23/2018 02:52 PM, Wei Liu wrote: > > On Fri, Apr 20, 2018 at 02:25:20PM +0300, Oleksandr Andrushchenko wrote: > > > > > the gntdev. > > > > > > > > > > I think this is generic enough that it could be implemented by a > > > > > device not tied to Xen. AFAICT the hyper_dma guys also wanted > > > > > something similar to this. > > > > You can't just wrap random userspace memory into a dma-buf. We've just > > > > had > > > > this discussion with kvm/qemu folks, who proposed just that, and after a > > > > bit of discussion they'll now try to have a driver which just wraps a > > > > memfd into a dma-buf. > > > So, we have to decide either we introduce a new driver > > > (say, under drivers/xen/xen-dma-buf) or extend the existing > > > gntdev/balloon to support dma-buf use-cases. > > > > > > Can anybody from Xen community express their preference here? > > > > > Oleksandr talked to me on IRC about this, he said a few IOCTLs need to > > be added to either existing drivers or a new driver. > > > > I went through this thread twice and skimmed through the relevant > > documents, but I couldn't see any obvious pros and cons for either > > approach. So I don't really have an opinion on this. > > > > But, assuming if implemented in existing drivers, those IOCTLs need to > > be added to different drivers, which means userspace program needs to > > write more code and get more handles, it would be slightly better to > > implement a new driver from that perspective. > If gntdev/balloon extension is still considered: > > All the IOCTLs will be in gntdev driver (in current xen-zcopy terminology): > - DRM_ICOTL_XEN_ZCOPY_DUMB_FROM_REFS > - DRM_IOCTL_XEN_ZCOPY_DUMB_TO_REFS > - DRM_IOCTL_XEN_ZCOPY_DUMB_WAIT_FREE s/DUMB/DMA_BUF/ please. This is generic dma-buf, it has nothing to do with the dumb scanout buffer support in the drm/gfx subsystem. This here can be used for any zcopy sharing among guests (as long as your endpoints understands dma-buf, which most relevant drivers do). -Daniel > > Balloon driver extension, which is needed for contiguous/DMA > buffers, will be to provide new *kernel API*, no UAPI is needed. > > > Wei. > Thank you, > Oleksandr > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.11] x86/spec_ctrl: Fix typo in ARCH_CAPS decode
>>> On 24.04.18 at 11:44, wrote: > Signed-off-by: Andrew Cooper Acked-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] RFC Xen signature verification for kexec
>>> On 24.04.18 at 12:13, wrote: > On Tue, Apr 24, 2018 at 10:46:38AM +0100, George Dunlap wrote: >> On Mon, Apr 23, 2018 at 11:33 AM, Jan Beulich wrote: >> On 23.04.18 at 12:25, wrote: >> >> On Mon, Apr 23, 2018 at 12:55:45AM -0600, Jan Beulich wrote: >> >>> >>> On 20.04.18 at 21:12, wrote: >> >>> > Two options for signature verification in Xen >> >>> > >> >>> > This proposal outlines two options under consideration for enhancing >> >>> > Xen to support signature verification of kexec loaded images. The >> >>> > first option is essentially to mirror Linux signature verification >> >>> > code into Xen. The second option utilizes components from sources >> >>> > other than Linux (for example, libgcrypt rather than linux/crypto). >> >>> > >> >>> > NOTE: An option to utilize dom0 kernel signature verification does not >> >>> > prevent the exploit as user space can invoke the hypercall directly, >> >>> > bypassing dom0. >> >>> >> >>> Not exactly - this option nevertheless exists, albeit is perhaps >> >>> unattractive: No user space component can issue hypercalls >> >>> directly, they always go through the privcmd driver. Hence the >> >>> driver cold snoop the kexec hypercall. >> >> >> >> Hmmm... Is not it a problem from security point of view for us in this >> >> case? It should not if dom0 kernel is signed. It have to be signed here. >> >> Just thinking a loud... >> > >> > I'm afraid I don't understand: If the Dom0 kernel isn't signed (or hasn't >> > been verified), the system is insecure in the first place. No reason to >> > bother measuring the kexec kernel then. >> >> I think you're both saying the same thing. >> >> FWIW I wouldn't mind coming up with a hypercall that the privcmd >> driver refuses to pass-through as-is, and having some way for the >> tools to ask the kernel to check the signature. > > I have a feeling that I should reformulate the question: How far the Xen > hypervisor trusts the privcmd driver? If the privcmd driver is signed > then at first sight there should not be a problem. However, we can be > more strict and require that (every? Daniel is running away...) hypercall > from privcmd to Xen should be verified somehow. Maybe I am overzealous but > I think that it make sense to discuss this now than later have problems. An analogy of this would be that every system call needs verifying by the OS, which imo would be insane. Once we established trust in a component, we _should_ trust it. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v8 1/9] x86/xpti: avoid copying L4 page table contents when possible
On 24/04/18 12:31, Tim Deegan wrote: > At 07:45 +0200 on 23 Apr (1524469545), Juergen Gross wrote: >> On 22/04/18 18:39, Tim Deegan wrote: >>> At 19:11 +0200 on 21 Apr (1524337893), Juergen Gross wrote: On 21/04/18 15:32, Tim Deegan wrote: > At 09:44 +0200 on 19 Apr (1524131080), Juergen Gross wrote: >> Another alternative would be to pass another flag to the callers to >> signal the need for a flush. This would require quite some modifications >> to shadow code I'd like to avoid, though. OTOH this way we could combine >> flushing the tlb and the root page tables. Tim, any preferences? > > This sounds a promising direction but it should be doabl without major > surgery to the shadow code. The shadow code already leaves old sl4es > visible (in TLBs) when it's safe to do so, so I think the right place > to hook this is on the receiving side of the TLB flush IPI. IOW as > long as: > - you copy the L4 on context switch; and > - you copy it on the TLB flush IPI is received > then you can rely on the existing TLB flush mechanisms to do what you > need. > And shadow doesn't have to behave differently from 'normal' PV MM. It is not so easy. The problem is that e.g. a page fault will flush the TLB entry for the page in question, but it won't lead to the L4 to be copied. >>> >>> Oh yes, I see; thanks for the explanation. It might be worth copying >>> what the hardware does here, and checking/propagating the relevant l4e >>> in the PV pagefault handler. >> >> While in the long run being an interesting option I'm not sure I want >> to go this route for 4.11. There might be nasty corner cases and I think >> such a lazy approach is much more error prone than doing explicit >> updates of the L4 table on the affected cpus in case of a modified >> entry. I think we should either do the explicit call of flush_mask() in >> shadow_set_l4e() or propagate the need for the flush up to the caller. > > FWIW, I disagree -- I think that having the fault handler DTRT and > relying on the existing, tested, TLB-flush logic is more likely to > work than introducing a new mechanism that _also_ has to catch every > possible l4e update. It should touch less code and be less likely to > break with later changes. And I think it would be better to do it > 'properly' now than to hope there's time to revisit it later. That > said, if Jan agrees that this way is OK, I'll quit grumbling and > review the shadow parts. :) Okay, thanks. > I think that setting the bits in shadow_set_l4e() is better than > having this leak out into all the callers. I'm happy to see that the > hunk in l4e_propagate_from_guest() has gone away too. > > Please move the shadow_set_l4e() hunk up so it's just after the write, > and before the general TLB flush logic. Okay. > Please move the logic into your code: the new function should take a > domain pointer and do all the filtering itself rather than have shadow > code be aware of what xpti is or why the domain's dirty-cpumask is > relevant. Okay. > It doesn't look like there's any check limiting this to PV guests, and > I think there should be, right? In my newest version it already is testing that. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [qemu-mainline test] 122357: tolerable FAIL - PUSHED
flight 122357 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/122357/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail like 122212 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 122212 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 122212 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 122212 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 122212 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 122212 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 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-xl-credit2 14 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-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 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-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass version targeted for testing: qemuu27e757e29cc79f3f104d2a84d17cdb3b4c11c8ff baseline version: qemuu38e83a71d02e026d4a6d0ab1ef9855c4924c2c68 Last test of basis 122212 2018-04-12 22:53:39 Z 11 days Failing since122328 2018-04-16 09:43:42 Z8 days3 attempts Testing same since 122357 2018-04-23 11:07:12 Z1 days1 attempts People who touched revisions under test: Alex Bennée Bastian Koppelmann Eduardo Habkost Emilio G. Cota Jason Wang Laurent Vivier Marc-André Lureau Max Reitz Michael S. Tsirkin Michael Tokarev Pavel Dovgalyuk Peter Maydell Philippe Mathieu-Daudé Richard Henderson Vladimir Sementsov-Ogievskiy Wanpeng Li jobs: build-amd64-xsm pass build-arm64-xsm pass build-armhf-xsm pass build-i3
Re: [Xen-devel] [PATCH 4/6] arm: add a small kconfig for Renesas RCar H3
On 24/04/2018 12:04, Andrii Anisov wrote: Hello Julien, On 24.04.18 13:06, Julien Grall wrote: I believe that passthrough should be kept as core Xen, it is small and quite tight to the rest of Xen. It might be. But I would be interested to know what would be the rationale to disable that. Do you foresee certification on IOMMU-less platform? I would give TI Jacinto6 as an example. It is an automotive focused SoC, has no IOMMU. BTW, I have a strong feeling that some J6 based automotive systems with XEN already reached production stage. I'm not really sure if they are certified, but anyway. So, yes, I'm pretty sure IOMMU-less platforms with XEN would require certification. Can you quantify what would be the cost of keeping that code around for IOMMU-less platform? Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] SVM MSRs issue
> While testing MSR vm_events, we've come accross some puzzling behaviour > while trying to follow the guest's MSR_LSTAR: it starts out as zero, > then it changes value before the first MSR write event, without going > through svm_msr_write_intercept(). The culprit seems to be a > svm_vmsave_pa() call. I should mention that this only happens with our introspection agent active, which calls XEN_DOMCTL_gethvmcontext_partial as soon as the guest is up. That is the call that ends up in hvm_save_one() and then further on to svm_vmsave_pa(). Starting a guest "normally" is fine, everything works as expected. Thanks, Razvan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC PATCH 1/1] xen: credit2: rb-tree for runqueues
On Tue, 2018-04-24 at 14:30 +0530, Praveen Kumar wrote: > Hi Dario, > Hi! > On Tuesday 17 April 2018 04:16 PM, Dario Faggioli wrote: > > On Tue, 2018-04-03 at 22:25 +0530, Praveen Kumar wrote: > > > > > > +if ( svc->credit < entry->credit ) > > > +node = &parent->rb_left; > > > +else > > > +node = &parent->rb_right; > > > + > > > +(*pos)++; > > > +} > > > +rb_link_node(&svc->runq_elem, parent, node); > > > +rb_insert_color(&svc->runq_elem, root); > > > +} > > > > > Wait, where's the part where we cache which element is the one with > > the > > highest credits? (And the same applies to the tree-removal > > function, of > > course.) > > > > In fact, we need a field for storing such a cache in the runqueue > > data > > structure as well, and we need to keep it updated. > > I thought of caching the left most node as done in rb_tree, but I > thought of taking an easy way to have things working and delaying > the > Linux rb_tree caching variant to be ported in next patch or so. > That is fine, as soon as the fact that you are not doing it right now, but planning to do it in another patch is stated somewhere (e.g., cover letter or a changelog). > > I would suggest we do not try to use the rb_*_cached() functions, > > and > > cache rightmost explicitly in runqueue_data. > > Ok, that sounds better, I will introduce an entry for rightmost > element > to be cached in runqueue_data > Also, lets port the Linux rb_tree cache variant as well ( probably > in > future we may use that ). > I'm not sure about this last part. I mean, I can see other uses of rb- trees, TBH, and ones where such caching would be useful. Still, I'll do the port when we actually decide to use the new functionallity (or when, e.g., we run into issues retro-fitting a Linux fix, etc). > > Err... is it? Isn't the leftmost element the one with the _least_ > > credits? It looks to me that we want rb_last(). > > > > And IAC, we don't want to have to traverse the tree to get the > > runnable > > vcpu with the highest credit, we want it available in O(1) time. > > > > That's why we want to cache it. > > Yes, it looks like an error. Will update the patch in v2. > Right. So, I think the main problem with this patch was this one, i.e., the fact that the runqueue was sorted in the wrong order. Then there is the lack of caching, for O(1) access to the head of the runqueue itself. As said, it is ok for that to come in its own patch of this series. It is also ok if this comes as a later patch, as soon as that is clearly stated. > Sure, let me have 3 series, first; Linux porting , second; rb_tree > changes which doesn't have caching and third; have rightmost node > cached. > I'd actually skip doing the porting right now... Although, in this case, it's not really my call, and others may have different a opinion. Regards, Dario -- <> (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Software Engineer @ SUSE https://www.suse.com/ signature.asc Description: This is a digitally signed message part ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 4/6] arm: add a small kconfig for Renesas RCar H3
Hello Julien, On 24.04.18 13:06, Julien Grall wrote: I believe that passthrough should be kept as core Xen, it is small and quite tight to the rest of Xen. It might be. But I would be interested to know what would be the rationale to disable that. Do you foresee certification on IOMMU-less platform? I would give TI Jacinto6 as an example. It is an automotive focused SoC, has no IOMMU. BTW, I have a strong feeling that some J6 based automotive systems with XEN already reached production stage. I'm not really sure if they are certified, but anyway. So, yes, I'm pretty sure IOMMU-less platforms with XEN would require certification. -- *Andrii Anisov* ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 02/10] xen/arm: Ignore write to GICD_ISACTIVERn registers (vgic-v2)
Hi Julien,
On Mon, Apr 23, 2018 at 1:15 PM, Julien Grall wrote:
> Hi Mirela,
>
>
> On 20/04/18 13:25, Mirela Simonovic wrote:
>>
>> Guests attempt to write into these registers on resume (for example
>> Linux).
>> Without this patch a data abort exception will be raised to the guest.
>> This patch handles the write access by ignoring it, but only if the value
>> to be written is zero. This should be fine because reading these registers
>> is already handled as 'read as zero'.
>>
>> Signed-off-by: Mirela Simonovic
>>
>> ---
>> CC: Stefano Stabellini
>> CC: Julien Grall
>> ---
>> Changes in v2:
>> - Write should be ignored only if the value to be written is zero
>> (in v1 the write was ignored regardless of the value)
>> ---
>> xen/arch/arm/vgic-v2.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
>> index 646d1f3d12..afd3e89883 100644
>> --- a/xen/arch/arm/vgic-v2.c
>> +++ b/xen/arch/arm/vgic-v2.c
>> @@ -488,7 +488,9 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v,
>> mmio_info_t *info,
>> printk(XENLOG_G_ERR
>> "%pv: vGICD: unhandled word write %#"PRIregister" to
>> ISACTIVER%d\n",
>> v, r, gicd_reg - GICD_ISACTIVER);
>> -return 0;
>> +if ( r != 0 )
>> +return 0;
>
>
> It would be better to move the check before the printk. So a warning is
> avoided when the guest is writing 0.
>
If we want to avoid printing a warning when the guest is writing 0
then the printk needs to be moved within the check. I guess this is
what you meant:
if ( r != 0 )
{
printk(XENLOG_G_ERR
"%pv: vGICD: unhandled word write %#"PRIregister" to ISACTIVER%d\n",
v, r, gicd_reg - GICD_ISACTIVER);
return 0;
}
goto write_ignore_32;
Please note that in the original patch where I ignored the write
regardless of the value I just followed how it is already done for
GICD_ICACTIVER.
For existing GICD_ICACTIVER case there is no check for the value to be
written and there is a warning printed.
Not checking the value seems fine to me but why is then a warning
printed? Should we suppress that print as well?
> Cheers,
>
>> +goto write_ignore_32;
>> case VRANGE32(GICD_ICACTIVER, GICD_ICACTIVERN):
>> printk(XENLOG_G_ERR
>>
>
> Cheers,
>
> --
> Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [distros-debian-snapshot test] 74637: tolerable FAIL
flight 74637 distros-debian-snapshot real [real] http://osstest.xs.citrite.net/~osstest/testlogs/logs/74637/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-i386-i386-daily-netboot-pvgrub 10 debian-di-install fail like 74630 test-amd64-i386-i386-weekly-netinst-pygrub 10 debian-di-install fail like 74630 test-amd64-amd64-i386-daily-netboot-pygrub 10 debian-di-install fail like 74630 test-amd64-i386-amd64-weekly-netinst-pygrub 10 debian-di-install fail like 74630 test-amd64-amd64-amd64-daily-netboot-pvgrub 11 guest-start fail like 74630 test-amd64-amd64-i386-weekly-netinst-pygrub 10 debian-di-install fail like 74630 test-amd64-amd64-amd64-weekly-netinst-pygrub 10 debian-di-install fail like 74630 test-amd64-amd64-amd64-current-netinst-pygrub 10 debian-di-install fail like 74630 test-armhf-armhf-armhf-daily-netboot-pygrub 10 debian-di-install fail like 74630 test-amd64-i386-i386-current-netinst-pygrub 10 debian-di-install fail like 74630 test-amd64-i386-amd64-current-netinst-pygrub 10 debian-di-install fail like 74630 test-amd64-amd64-i386-current-netinst-pygrub 10 debian-di-install fail like 74630 baseline version: flight 74630 jobs: build-amd64 pass build-armhf pass build-i386 pass build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-amd64-daily-netboot-pvgrub fail test-amd64-i386-i386-daily-netboot-pvgrubfail test-amd64-i386-amd64-daily-netboot-pygrub pass test-armhf-armhf-armhf-daily-netboot-pygrub fail test-amd64-amd64-i386-daily-netboot-pygrub fail test-amd64-amd64-amd64-current-netinst-pygrubfail test-amd64-i386-amd64-current-netinst-pygrub fail test-amd64-amd64-i386-current-netinst-pygrub fail test-amd64-i386-i386-current-netinst-pygrub fail test-amd64-amd64-amd64-weekly-netinst-pygrub fail test-amd64-i386-amd64-weekly-netinst-pygrub fail test-amd64-amd64-i386-weekly-netinst-pygrub fail test-amd64-i386-i386-weekly-netinst-pygrub fail sg-report-flight on osstest.xs.citrite.net logs: /home/osstest/logs images: /home/osstest/images Logs, config files, etc. are available at http://osstest.xs.citrite.net/~osstest/testlogs/logs Test harness code can be found at http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary Push not applicable. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v8 1/9] x86/xpti: avoid copying L4 page table contents when possible
At 07:45 +0200 on 23 Apr (1524469545), Juergen Gross wrote: > On 22/04/18 18:39, Tim Deegan wrote: > > At 19:11 +0200 on 21 Apr (1524337893), Juergen Gross wrote: > >> On 21/04/18 15:32, Tim Deegan wrote: > >>> At 09:44 +0200 on 19 Apr (1524131080), Juergen Gross wrote: > Another alternative would be to pass another flag to the callers to > signal the need for a flush. This would require quite some modifications > to shadow code I'd like to avoid, though. OTOH this way we could combine > flushing the tlb and the root page tables. Tim, any preferences? > >>> > >>> This sounds a promising direction but it should be doabl without major > >>> surgery to the shadow code. The shadow code already leaves old sl4es > >>> visible (in TLBs) when it's safe to do so, so I think the right place > >>> to hook this is on the receiving side of the TLB flush IPI. IOW as > >>> long as: > >>> - you copy the L4 on context switch; and > >>> - you copy it on the TLB flush IPI is received > >>> then you can rely on the existing TLB flush mechanisms to do what you > >>> need. > >>> And shadow doesn't have to behave differently from 'normal' PV MM. > >> > >> It is not so easy. The problem is that e.g. a page fault will flush the > >> TLB entry for the page in question, but it won't lead to the L4 to be > >> copied. > > > > Oh yes, I see; thanks for the explanation. It might be worth copying > > what the hardware does here, and checking/propagating the relevant l4e > > in the PV pagefault handler. > > While in the long run being an interesting option I'm not sure I want > to go this route for 4.11. There might be nasty corner cases and I think > such a lazy approach is much more error prone than doing explicit > updates of the L4 table on the affected cpus in case of a modified > entry. I think we should either do the explicit call of flush_mask() in > shadow_set_l4e() or propagate the need for the flush up to the caller. FWIW, I disagree -- I think that having the fault handler DTRT and relying on the existing, tested, TLB-flush logic is more likely to work than introducing a new mechanism that _also_ has to catch every possible l4e update. It should touch less code and be less likely to break with later changes. And I think it would be better to do it 'properly' now than to hope there's time to revisit it later. That said, if Jan agrees that this way is OK, I'll quit grumbling and review the shadow parts. :) I think that setting the bits in shadow_set_l4e() is better than having this leak out into all the callers. I'm happy to see that the hunk in l4e_propagate_from_guest() has gone away too. Please move the shadow_set_l4e() hunk up so it's just after the write, and before the general TLB flush logic. Please move the logic into your code: the new function should take a domain pointer and do all the filtering itself rather than have shadow code be aware of what xpti is or why the domain's dirty-cpumask is relevant. It doesn't look like there's any check limiting this to PV guests, and I think there should be, right? Cheers, Tim. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/1] drm/xen-zcopy: Add Xen zero-copy helper DRM driver
On 24/04/18 12:14, Oleksandr Andrushchenko wrote: > On 04/24/2018 01:01 PM, Wei Liu wrote: >> On Tue, Apr 24, 2018 at 11:08:41AM +0200, Juergen Gross wrote: >>> On 24/04/18 11:03, Oleksandr Andrushchenko wrote: On 04/24/2018 11:40 AM, Juergen Gross wrote: > On 24/04/18 10:07, Oleksandr Andrushchenko wrote: >> On 04/24/2018 10:51 AM, Juergen Gross wrote: >>> On 24/04/18 07:43, Oleksandr Andrushchenko wrote: On 04/24/2018 01:41 AM, Boris Ostrovsky wrote: > On 04/23/2018 08:10 AM, Oleksandr Andrushchenko wrote: >> On 04/23/2018 02:52 PM, Wei Liu wrote: >>> On Fri, Apr 20, 2018 at 02:25:20PM +0300, Oleksandr >>> Andrushchenko >>> wrote: >> the gntdev. >> >> I think this is generic enough that it could be >> implemented by a >> device not tied to Xen. AFAICT the hyper_dma guys also wanted >> something similar to this. > You can't just wrap random userspace memory into a dma-buf. > We've > just had > this discussion with kvm/qemu folks, who proposed just > that, and > after a > bit of discussion they'll now try to have a driver which just > wraps a > memfd into a dma-buf. So, we have to decide either we introduce a new driver (say, under drivers/xen/xen-dma-buf) or extend the existing gntdev/balloon to support dma-buf use-cases. Can anybody from Xen community express their preference here? >>> Oleksandr talked to me on IRC about this, he said a few IOCTLs >>> need to >>> be added to either existing drivers or a new driver. >>> >>> I went through this thread twice and skimmed through the >>> relevant >>> documents, but I couldn't see any obvious pros and cons for >>> either >>> approach. So I don't really have an opinion on this. >>> >>> But, assuming if implemented in existing drivers, those IOCTLs >>> need to >>> be added to different drivers, which means userspace program >>> needs to >>> write more code and get more handles, it would be slightly >>> better to >>> implement a new driver from that perspective. >> If gntdev/balloon extension is still considered: >> >> All the IOCTLs will be in gntdev driver (in current xen-zcopy >> terminology): >> - DRM_ICOTL_XEN_ZCOPY_DUMB_FROM_REFS >> - DRM_IOCTL_XEN_ZCOPY_DUMB_TO_REFS >> - DRM_IOCTL_XEN_ZCOPY_DUMB_WAIT_FREE >> >> Balloon driver extension, which is needed for contiguous/DMA >> buffers, will be to provide new *kernel API*, no UAPI is needed. >> > So I am obviously a bit late to this thread, but why do you need > to add > new ioctls to gntdev and balloon? Doesn't this driver manage to do > what > you want without any extensions? 1. I only (may) need to add IOCTLs to gntdev 2. balloon driver needs to be extended, so it can allocate contiguous (DMA) memory, not IOCTLs/UAPI here, all lives in the kernel. 3. The reason I need to extend gnttab with new IOCTLs is to provide new functionality to create a dma-buf from grant references and to produce grant references for a dma-buf. This is what I have as UAPI description for xen-zcopy driver: 1. DRM_IOCTL_XEN_ZCOPY_DUMB_FROM_REFS This will create a DRM dumb buffer from grant references provided by the frontend. The intended usage is: - Frontend - creates a dumb/display buffer and allocates memory - grants foreign access to the buffer pages - passes granted references to the backend - Backend - issues DRM_XEN_ZCOPY_DUMB_FROM_REFS ioctl to map granted references and create a dumb buffer - requests handle to fd conversion via DRM_IOCTL_PRIME_HANDLE_TO_FD - requests real HW driver/consumer to import the PRIME buffer with DRM_IOCTL_PRIME_FD_TO_HANDLE - uses handle returned by the real HW driver - at the end: o closes real HW driver's handle with DRM_IOCTL_GEM_CLOSE o closes zero-copy driver's handle with DRM_IOCTL_GEM_CLOSE o closes file descriptor of the exported buffer 2. DRM_IOCTL_XEN_ZCOPY_DUMB_TO_REFS This will grant references to a dumb/display buffer's memory provided by the backend. The intended usage is: - Frontend - requests backend to allocate dumb
Re: [Xen-devel] [PATCH 0/1] drm/xen-zcopy: Add Xen zero-copy helper DRM driver
On 04/24/2018 01:01 PM, Wei Liu wrote: On Tue, Apr 24, 2018 at 11:08:41AM +0200, Juergen Gross wrote: On 24/04/18 11:03, Oleksandr Andrushchenko wrote: On 04/24/2018 11:40 AM, Juergen Gross wrote: On 24/04/18 10:07, Oleksandr Andrushchenko wrote: On 04/24/2018 10:51 AM, Juergen Gross wrote: On 24/04/18 07:43, Oleksandr Andrushchenko wrote: On 04/24/2018 01:41 AM, Boris Ostrovsky wrote: On 04/23/2018 08:10 AM, Oleksandr Andrushchenko wrote: On 04/23/2018 02:52 PM, Wei Liu wrote: On Fri, Apr 20, 2018 at 02:25:20PM +0300, Oleksandr Andrushchenko wrote: the gntdev. I think this is generic enough that it could be implemented by a device not tied to Xen. AFAICT the hyper_dma guys also wanted something similar to this. You can't just wrap random userspace memory into a dma-buf. We've just had this discussion with kvm/qemu folks, who proposed just that, and after a bit of discussion they'll now try to have a driver which just wraps a memfd into a dma-buf. So, we have to decide either we introduce a new driver (say, under drivers/xen/xen-dma-buf) or extend the existing gntdev/balloon to support dma-buf use-cases. Can anybody from Xen community express their preference here? Oleksandr talked to me on IRC about this, he said a few IOCTLs need to be added to either existing drivers or a new driver. I went through this thread twice and skimmed through the relevant documents, but I couldn't see any obvious pros and cons for either approach. So I don't really have an opinion on this. But, assuming if implemented in existing drivers, those IOCTLs need to be added to different drivers, which means userspace program needs to write more code and get more handles, it would be slightly better to implement a new driver from that perspective. If gntdev/balloon extension is still considered: All the IOCTLs will be in gntdev driver (in current xen-zcopy terminology): - DRM_ICOTL_XEN_ZCOPY_DUMB_FROM_REFS - DRM_IOCTL_XEN_ZCOPY_DUMB_TO_REFS - DRM_IOCTL_XEN_ZCOPY_DUMB_WAIT_FREE Balloon driver extension, which is needed for contiguous/DMA buffers, will be to provide new *kernel API*, no UAPI is needed. So I am obviously a bit late to this thread, but why do you need to add new ioctls to gntdev and balloon? Doesn't this driver manage to do what you want without any extensions? 1. I only (may) need to add IOCTLs to gntdev 2. balloon driver needs to be extended, so it can allocate contiguous (DMA) memory, not IOCTLs/UAPI here, all lives in the kernel. 3. The reason I need to extend gnttab with new IOCTLs is to provide new functionality to create a dma-buf from grant references and to produce grant references for a dma-buf. This is what I have as UAPI description for xen-zcopy driver: 1. DRM_IOCTL_XEN_ZCOPY_DUMB_FROM_REFS This will create a DRM dumb buffer from grant references provided by the frontend. The intended usage is: - Frontend - creates a dumb/display buffer and allocates memory - grants foreign access to the buffer pages - passes granted references to the backend - Backend - issues DRM_XEN_ZCOPY_DUMB_FROM_REFS ioctl to map granted references and create a dumb buffer - requests handle to fd conversion via DRM_IOCTL_PRIME_HANDLE_TO_FD - requests real HW driver/consumer to import the PRIME buffer with DRM_IOCTL_PRIME_FD_TO_HANDLE - uses handle returned by the real HW driver - at the end: o closes real HW driver's handle with DRM_IOCTL_GEM_CLOSE o closes zero-copy driver's handle with DRM_IOCTL_GEM_CLOSE o closes file descriptor of the exported buffer 2. DRM_IOCTL_XEN_ZCOPY_DUMB_TO_REFS This will grant references to a dumb/display buffer's memory provided by the backend. The intended usage is: - Frontend - requests backend to allocate dumb/display buffer and grant references to its pages - Backend - requests real HW driver to create a dumb with DRM_IOCTL_MODE_CREATE_DUMB - requests handle to fd conversion via DRM_IOCTL_PRIME_HANDLE_TO_FD - requests zero-copy driver to import the PRIME buffer with DRM_IOCTL_PRIME_FD_TO_HANDLE - issues DRM_XEN_ZCOPY_DUMB_TO_REFS ioctl to grant references to the buffer's memory. - passes grant references to the frontend - at the end: - closes zero-copy driver's handle with DRM_IOCTL_GEM_CLOSE - closes real HW driver's handle with DRM_IOCTL_GEM_CLOSE - closes file descriptor of the imported buffer 3. DRM_XEN_ZCOPY_DUMB_WAIT_FREE This will block until the dumb buffer with the wait handle provided be freed: this is needed for synchronization between frontend and backend in case frontend provides grant references of the buffer via DRM_XEN_ZCOPY_DUMB_FROM_REFS IOCTL and which must be released before backend replies with XENDISPL_OP_DBUF_DESTROY response. wait_handle must be the same value returned while calling DRM_XEN_ZCOPY_DUMB_FROM_REFS IOCTL. So,
Re: [Xen-devel] RFC Xen signature verification for kexec
On Tue, Apr 24, 2018 at 10:46:38AM +0100, George Dunlap wrote: > On Mon, Apr 23, 2018 at 11:33 AM, Jan Beulich wrote: > On 23.04.18 at 12:25, wrote: > >> On Mon, Apr 23, 2018 at 12:55:45AM -0600, Jan Beulich wrote: > >>> >>> On 20.04.18 at 21:12, wrote: > >>> > Two options for signature verification in Xen > >>> > > >>> > This proposal outlines two options under consideration for enhancing > >>> > Xen to support signature verification of kexec loaded images. The > >>> > first option is essentially to mirror Linux signature verification > >>> > code into Xen. The second option utilizes components from sources > >>> > other than Linux (for example, libgcrypt rather than linux/crypto). > >>> > > >>> > NOTE: An option to utilize dom0 kernel signature verification does not > >>> > prevent the exploit as user space can invoke the hypercall directly, > >>> > bypassing dom0. > >>> > >>> Not exactly - this option nevertheless exists, albeit is perhaps > >>> unattractive: No user space component can issue hypercalls > >>> directly, they always go through the privcmd driver. Hence the > >>> driver cold snoop the kexec hypercall. > >> > >> Hmmm... Is not it a problem from security point of view for us in this > >> case? It should not if dom0 kernel is signed. It have to be signed here. > >> Just thinking a loud... > > > > I'm afraid I don't understand: If the Dom0 kernel isn't signed (or hasn't > > been verified), the system is insecure in the first place. No reason to > > bother measuring the kexec kernel then. > > I think you're both saying the same thing. > > FWIW I wouldn't mind coming up with a hypercall that the privcmd > driver refuses to pass-through as-is, and having some way for the > tools to ask the kernel to check the signature. I have a feeling that I should reformulate the question: How far the Xen hypervisor trusts the privcmd driver? If the privcmd driver is signed then at first sight there should not be a problem. However, we can be more strict and require that (every? Daniel is running away...) hypercall from privcmd to Xen should be verified somehow. Maybe I am overzealous but I think that it make sense to discuss this now than later have problems. Daniel ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 4/6] arm: add a small kconfig for Renesas RCar H3
On 04/24/2018 10:52 AM, Andrii Anisov wrote: Hello Julien, Hi Andrii, On 24.04.18 12:01, Julien Grall wrote: You can't unselect HAS_PASSTHROUGH support. Given that you are going to have passthrough in the future, I don't much see the point to try to allow that option to be disabled. If we are speaking of R-Car Gen3, you might be right. We are targeting IPMMU support upstreamed. What about IOMMU-less platforms? For everything you disable, you double the number of configuration possible. So we have to weight the benefits of disabling some code. I believe that passthrough should be kept as core Xen, it is small and quite tight to the rest of Xen. But I would be interested to know what would be the rationale to disable that. Do you foresee certification on IOMMU-less platform? Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] SVM MSRs issue
Hello,
While testing MSR vm_events, we've come accross some puzzling behaviour
while trying to follow the guest's MSR_LSTAR: it starts out as zero,
then it changes value before the first MSR write event, without going
through svm_msr_write_intercept(). The culprit seems to be a
svm_vmsave_pa() call, coming though this callstack:
(XEN) Xen call trace:
(XEN)[] svm.c#svm_sync_vmcb+0xa5/0xb4
(XEN)[] svm.c#svm_get_segment_register+0x6a/0x12b
(XEN)[] hvm_get_segment_register+0x19/0xed
(XEN)[] hvm.c#hvm_save_cpu_ctxt+0x218/0x4ef
(XEN)[] hvm_save_one+0xcb/0x249
(XEN)[] arch_do_domctl+0x89f/0x26c7
(XEN)[] do_domctl+0x17c0/0x1be4
(XEN)[] pv_hypercall+0x1f4/0x440
(XEN)[] x86_64/entry.S#test_all_events+0/0x30
From what I can tell, this is effectively a plain VMSAVE:
36 static inline void svm_vmsave_pa(paddr_t vmcb)
37 {
38 asm volatile (
39 ".byte 0x0f,0x01,0xdb" /* vmsave */
40 : : "a" (vmcb) : "memory" );
41 }
and using GNU Linux' rdmsr, it would appear that the rogue value is a
_host_ MSR_LSTAR value.
So at least sometimes, the above callstack overwrites v->vmcb values -
specifically when we're in the host's context.
We're trying to understand what the best fix would be here. Suggestions
from the SVM maintainers are appreciated.
Thanks,
Razvan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/1] drm/xen-zcopy: Add Xen zero-copy helper DRM driver
On Tue, Apr 24, 2018 at 11:08:41AM +0200, Juergen Gross wrote: > On 24/04/18 11:03, Oleksandr Andrushchenko wrote: > > On 04/24/2018 11:40 AM, Juergen Gross wrote: > >> On 24/04/18 10:07, Oleksandr Andrushchenko wrote: > >>> On 04/24/2018 10:51 AM, Juergen Gross wrote: > On 24/04/18 07:43, Oleksandr Andrushchenko wrote: > > On 04/24/2018 01:41 AM, Boris Ostrovsky wrote: > >> On 04/23/2018 08:10 AM, Oleksandr Andrushchenko wrote: > >>> On 04/23/2018 02:52 PM, Wei Liu wrote: > On Fri, Apr 20, 2018 at 02:25:20PM +0300, Oleksandr Andrushchenko > wrote: > >>> the gntdev. > >>> > >>> I think this is generic enough that it could be implemented by a > >>> device not tied to Xen. AFAICT the hyper_dma guys also wanted > >>> something similar to this. > >> You can't just wrap random userspace memory into a dma-buf. We've > >> just had > >> this discussion with kvm/qemu folks, who proposed just that, and > >> after a > >> bit of discussion they'll now try to have a driver which just > >> wraps a > >> memfd into a dma-buf. > > So, we have to decide either we introduce a new driver > > (say, under drivers/xen/xen-dma-buf) or extend the existing > > gntdev/balloon to support dma-buf use-cases. > > > > Can anybody from Xen community express their preference here? > > > Oleksandr talked to me on IRC about this, he said a few IOCTLs > need to > be added to either existing drivers or a new driver. > > I went through this thread twice and skimmed through the relevant > documents, but I couldn't see any obvious pros and cons for either > approach. So I don't really have an opinion on this. > > But, assuming if implemented in existing drivers, those IOCTLs > need to > be added to different drivers, which means userspace program > needs to > write more code and get more handles, it would be slightly > better to > implement a new driver from that perspective. > >>> If gntdev/balloon extension is still considered: > >>> > >>> All the IOCTLs will be in gntdev driver (in current xen-zcopy > >>> terminology): > >>> - DRM_ICOTL_XEN_ZCOPY_DUMB_FROM_REFS > >>> - DRM_IOCTL_XEN_ZCOPY_DUMB_TO_REFS > >>> - DRM_IOCTL_XEN_ZCOPY_DUMB_WAIT_FREE > >>> > >>> Balloon driver extension, which is needed for contiguous/DMA > >>> buffers, will be to provide new *kernel API*, no UAPI is needed. > >>> > >> So I am obviously a bit late to this thread, but why do you need > >> to add > >> new ioctls to gntdev and balloon? Doesn't this driver manage to do > >> what > >> you want without any extensions? > > 1. I only (may) need to add IOCTLs to gntdev > > 2. balloon driver needs to be extended, so it can allocate > > contiguous (DMA) memory, not IOCTLs/UAPI here, all lives > > in the kernel. > > 3. The reason I need to extend gnttab with new IOCTLs is to > > provide new functionality to create a dma-buf from grant references > > and to produce grant references for a dma-buf. This is what I have as > > UAPI > > description for xen-zcopy driver: > > > > 1. DRM_IOCTL_XEN_ZCOPY_DUMB_FROM_REFS > > This will create a DRM dumb buffer from grant references provided > > by the frontend. The intended usage is: > > - Frontend > > - creates a dumb/display buffer and allocates memory > > - grants foreign access to the buffer pages > > - passes granted references to the backend > > - Backend > > - issues DRM_XEN_ZCOPY_DUMB_FROM_REFS ioctl to map > > granted references and create a dumb buffer > > - requests handle to fd conversion via > > DRM_IOCTL_PRIME_HANDLE_TO_FD > > - requests real HW driver/consumer to import the PRIME buffer > > with > > DRM_IOCTL_PRIME_FD_TO_HANDLE > > - uses handle returned by the real HW driver > > - at the end: > > o closes real HW driver's handle with DRM_IOCTL_GEM_CLOSE > > o closes zero-copy driver's handle with DRM_IOCTL_GEM_CLOSE > > o closes file descriptor of the exported buffer > > > > 2. DRM_IOCTL_XEN_ZCOPY_DUMB_TO_REFS > > This will grant references to a dumb/display buffer's memory > > provided by > > the > > backend. The intended usage is: > > - Frontend > > - requests backend to allocate dumb/display buffer and grant > > references > > to its pages > > - Backend > > - requests real HW driver to create a dumb with > > DRM_IOCTL_MODE_CREATE_DUMB > > - requests handle to fd conversion via > > DRM_IOCTL_PRIME_HANDLE_TO_FD > > - requests ze
Re: [Xen-devel] [PATCH 4/6] arm: add a small kconfig for Renesas RCar H3
Hello Julien, On 24.04.18 12:01, Julien Grall wrote: You can't unselect HAS_PASSTHROUGH support. Given that you are going to have passthrough in the future, I don't much see the point to try to allow that option to be disabled. If we are speaking of R-Car Gen3, you might be right. We are targeting IPMMU support upstreamed. What about IOMMU-less platforms? -- *Andrii Anisov* ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.11] x86/spec_ctrl: Fix typo in ARCH_CAPS decode
On 24/04/18 11:44, Andrew Cooper wrote: > Signed-off-by: Andrew Cooper Release-acked-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] RFC Xen signature verification for kexec
On Mon, Apr 23, 2018 at 11:33 AM, Jan Beulich wrote: On 23.04.18 at 12:25, wrote: >> On Mon, Apr 23, 2018 at 12:55:45AM -0600, Jan Beulich wrote: >>> >>> On 20.04.18 at 21:12, wrote: >>> > Two options for signature verification in Xen >>> > >>> > This proposal outlines two options under consideration for enhancing >>> > Xen to support signature verification of kexec loaded images. The >>> > first option is essentially to mirror Linux signature verification >>> > code into Xen. The second option utilizes components from sources >>> > other than Linux (for example, libgcrypt rather than linux/crypto). >>> > >>> > NOTE: An option to utilize dom0 kernel signature verification does not >>> > prevent the exploit as user space can invoke the hypercall directly, >>> > bypassing dom0. >>> >>> Not exactly - this option nevertheless exists, albeit is perhaps >>> unattractive: No user space component can issue hypercalls >>> directly, they always go through the privcmd driver. Hence the >>> driver cold snoop the kexec hypercall. >> >> Hmmm... Is not it a problem from security point of view for us in this >> case? It should not if dom0 kernel is signed. It have to be signed here. >> Just thinking a loud... > > I'm afraid I don't understand: If the Dom0 kernel isn't signed (or hasn't > been verified), the system is insecure in the first place. No reason to > bother measuring the kexec kernel then. I think you're both saying the same thing. FWIW I wouldn't mind coming up with a hypercall that the privcmd driver refuses to pass-through as-is, and having some way for the tools to ask the kernel to check the signature. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC v2 6/9] libxl: Have QEMU save its state to a file descriptor
On Mon, Apr 16, 2018 at 06:32:24PM +0100, Anthony PERARD wrote: > In case QEMU have restricted access to the system, open the file for it, > and QEMU will save its state to this file descritor. > > Signed-off-by: Anthony PERARD Acked-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-4.11] x86/spec_ctrl: Fix typo in ARCH_CAPS decode
Signed-off-by: Andrew Cooper --- CC: Jan Beulich CC: Juergen Gross --- xen/arch/x86/spec_ctrl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c index bab8595..fab3c1d 100644 --- a/xen/arch/x86/spec_ctrl.c +++ b/xen/arch/x86/spec_ctrl.c @@ -119,7 +119,7 @@ static void __init print_details(enum ind_thunk thunk) (e8b & cpufeat_mask(X86_FEATURE_IBPB)) ? " IBPB" : "", (caps & ARCH_CAPABILITIES_IBRS_ALL) ? " IBRS_ALL" : "", (caps & ARCH_CAPABILITIES_RDCL_NO) ? " RDCL_NO" : "", - (caps & ARCH_CAPS_RSBA) ? " RBSA" : ""); + (caps & ARCH_CAPS_RSBA) ? " RSBA" : ""); /* Compiled-in support which pertains to BTI mitigations. */ if ( IS_ENABLED(CONFIG_INDIRECT_THUNK) ) -- 2.1.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-4.10-testing test] 122356: regressions - FAIL
flight 122356 xen-4.10-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/122356/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemut-win7-amd64 7 xen-boot fail REGR. vs. 122255 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 16 guest-localmigrate/x10 fail REGR. vs. 122255 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-rumprun-amd64 17 rumprun-demo-xenstorels/xenstorels.repeat fail REGR. vs. 122255 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-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-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-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-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-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop 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-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-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-win10-i386 10 windows-installfail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail 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 version targeted for testing: xen 656c14780c5c69ed8289b1f41fcdf1c84446bbba baseline version: xen 8d37ee1d101248ba9cf44d79352ade3b376db55c Last test of basis 122255 2018-04-13 14:59:05 Z 10 days Testing same since 122356 2018-04-23 11:06:42 Z0 days
