* [xen-unstable test] 78395: regressions - FAIL
@ 2016-01-18 12:10 osstest service owner
2016-01-18 12:55 ` Jan Beulich
0 siblings, 1 reply; 16+ messages in thread
From: osstest service owner @ 2016-01-18 12:10 UTC (permalink / raw)
To: xen-devel, osstest-admin
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 12247 bytes --]
flight 78395 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/78395/
Regressions :-(
Tests which did not succeed and are blocking,
including tests which could not be run:
test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 5 xen-install fail REGR. vs. 77892
test-amd64-amd64-xl-xsm 5 xen-install fail REGR. vs. 77892
test-amd64-i386-xl-xsm 5 xen-install fail REGR. vs. 77892
test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 5 xen-install fail REGR. vs. 77892
test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 5 xen-install fail REGR. vs. 77892
test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 5 xen-install fail REGR. vs. 77892
test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 5 xen-install fail REGR. vs. 77892
test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 5 xen-install fail REGR. vs. 77892
test-armhf-armhf-xl-xsm 6 xen-boot fail REGR. vs. 77892
test-amd64-i386-xl-qemut-win7-amd64 9 windows-install fail REGR. vs. 77892
test-amd64-i386-xl-qemuu-win7-amd64 9 windows-install fail REGR. vs. 77892
Tests which are failing intermittently (not blocking):
test-armhf-armhf-xl 17 guest-destroy fail pass in 78343
Regressions which are regarded as allowable (not blocking):
test-amd64-i386-libvirt-xsm 5 xen-install fail REGR. vs. 77892
test-amd64-amd64-libvirt-xsm 5 xen-install fail REGR. vs. 77892
test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 5 xen-install fail REGR. vs. 77892
test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 5 xen-install fail REGR. vs. 77892
test-armhf-armhf-libvirt-xsm 6 xen-boot fail REGR. vs. 77892
test-armhf-armhf-xl-rtds 15 guest-start/debian.repeat fail blocked in 77892
test-armhf-armhf-xl-rtds 9 debian-install fail in 78343 like 77892
test-amd64-i386-rumpuserxen-i386 10 guest-start fail like 77892
test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 77892
test-amd64-amd64-libvirt-vhd 9 debian-di-install fail like 77892
Tests which did not succeed, but are not blocking:
test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass
test-amd64-amd64-xl-pvh-intel 11 guest-start fail never pass
test-armhf-armhf-libvirt 14 guest-saverestore fail never pass
test-armhf-armhf-libvirt 12 migrate-support-check fail never pass
test-amd64-amd64-libvirt 12 migrate-support-check fail never pass
test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2 fail never pass
test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail never pass
test-amd64-i386-libvirt 12 migrate-support-check fail never pass
test-armhf-armhf-xl-credit2 13 saverestore-support-check fail never pass
test-armhf-armhf-xl-credit2 12 migrate-support-check fail never pass
test-armhf-armhf-xl 12 migrate-support-check fail never pass
test-armhf-armhf-xl 13 saverestore-support-check fail never pass
test-armhf-armhf-xl-cubietruck 12 migrate-support-check fail never pass
test-armhf-armhf-xl-cubietruck 13 saverestore-support-check fail never pass
test-armhf-armhf-xl-multivcpu 13 saverestore-support-check fail never pass
test-armhf-armhf-xl-multivcpu 12 migrate-support-check fail never pass
test-armhf-armhf-xl-rtds 13 saverestore-support-check fail never pass
test-armhf-armhf-xl-rtds 12 migrate-support-check fail never pass
test-armhf-armhf-libvirt-raw 13 guest-saverestore fail never pass
test-armhf-armhf-libvirt-raw 11 migrate-support-check fail never pass
test-armhf-armhf-libvirt-qcow2 11 migrate-support-check fail never pass
test-armhf-armhf-libvirt-qcow2 13 guest-saverestore fail never pass
test-armhf-armhf-xl-arndale 12 migrate-support-check fail never pass
test-armhf-armhf-xl-arndale 13 saverestore-support-check fail never pass
test-armhf-armhf-xl-vhd 9 debian-di-install fail never pass
version targeted for testing:
xen 7a1d1becf020f1fa511692bd97d50402588ef28d
baseline version:
xen f7347a282420a5edc74afb31e7c42c2765f24de5
Last test of basis 77892 2016-01-12 11:30:40 Z 6 days
Failing since 77945 2016-01-13 04:01:14 Z 5 days 9 attempts
Testing same since 78210 2016-01-15 22:45:42 Z 2 days 4 attempts
------------------------------------------------------------
People who touched revisions under test:
Andrew Cooper <andrew.cooper3@citrix.com>
Boris Ostrovsky <boris.ostrovsky@oracle.com>
Brendan Gregg <bgregg@netflix.com>
Daniel De Graaf <dgdegra@tycho.nsa.gov>
Doug Goldstein <cardoe@cardoe.com>
Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Haozhong Zhang <haozhong.zhang@intel.com>
Ian Campbell <ian.campbell@citrix.com>
Jan Beulich <jbeulich@suse.com>
Juergen Gross <jgross@suse.com>
Kevin Tian <kevin.tian@intel.com>
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Paul Durrant <paul.durrant@citrix.com>
Razvan Cojocaru <rcojocaru@bitdefender.com>
Roger Pau Monne <roger.pau@citrix.com>
Roger Pau Monné <roger.pau@citrix.com>
Tamas K Lengyel <tamas@tklengyel.com>
Wei Liu <wei.liu2@citrix.com>
Wei Liu <wei.liu2@citrix.com> for tools bits
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-oldkern pass
build-i386-oldkern pass
build-amd64-prev pass
build-i386-prev pass
build-amd64-pvops pass
build-armhf-pvops pass
build-i386-pvops pass
build-amd64-rumpuserxen pass
build-i386-rumpuserxen pass
test-amd64-amd64-xl pass
test-armhf-armhf-xl fail
test-amd64-i386-xl pass
test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm fail
test-amd64-i386-xl-qemut-debianhvm-amd64-xsm fail
test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm fail
test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm fail
test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm fail
test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm fail
test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm fail
test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm fail
test-amd64-amd64-libvirt-xsm fail
test-armhf-armhf-libvirt-xsm fail
test-amd64-i386-libvirt-xsm fail
test-amd64-amd64-xl-xsm fail
test-armhf-armhf-xl-xsm fail
test-amd64-i386-xl-xsm fail
test-amd64-amd64-qemuu-nested-amd fail
test-amd64-amd64-xl-pvh-amd fail
test-amd64-i386-qemut-rhel6hvm-amd pass
test-amd64-i386-qemuu-rhel6hvm-amd pass
test-amd64-amd64-xl-qemut-debianhvm-amd64 pass
test-amd64-i386-xl-qemut-debianhvm-amd64 pass
test-amd64-amd64-xl-qemuu-debianhvm-amd64 pass
test-amd64-i386-xl-qemuu-debianhvm-amd64 pass
test-amd64-i386-freebsd10-amd64 pass
test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
test-amd64-i386-xl-qemuu-ovmf-amd64 pass
test-amd64-amd64-rumpuserxen-amd64 pass
test-amd64-amd64-xl-qemut-win7-amd64 fail
test-amd64-i386-xl-qemut-win7-amd64 fail
test-amd64-amd64-xl-qemuu-win7-amd64 fail
test-amd64-i386-xl-qemuu-win7-amd64 fail
test-armhf-armhf-xl-arndale pass
test-amd64-amd64-xl-credit2 pass
test-armhf-armhf-xl-credit2 pass
test-armhf-armhf-xl-cubietruck pass
test-amd64-i386-freebsd10-i386 pass
test-amd64-i386-rumpuserxen-i386 fail
test-amd64-amd64-qemuu-nested-intel pass
test-amd64-amd64-xl-pvh-intel fail
test-amd64-i386-qemut-rhel6hvm-intel pass
test-amd64-i386-qemuu-rhel6hvm-intel pass
test-amd64-amd64-libvirt pass
test-armhf-armhf-libvirt fail
test-amd64-i386-libvirt pass
test-amd64-amd64-migrupgrade pass
test-amd64-i386-migrupgrade pass
test-amd64-amd64-xl-multivcpu pass
test-armhf-armhf-xl-multivcpu pass
test-amd64-amd64-pair pass
test-amd64-i386-pair pass
test-amd64-amd64-libvirt-pair pass
test-amd64-i386-libvirt-pair pass
test-amd64-amd64-amd64-pvgrub pass
test-amd64-amd64-i386-pvgrub pass
test-amd64-amd64-pygrub pass
test-armhf-armhf-libvirt-qcow2 fail
test-amd64-amd64-xl-qcow2 pass
test-armhf-armhf-libvirt-raw fail
test-amd64-i386-xl-raw pass
test-amd64-amd64-xl-rtds pass
test-armhf-armhf-xl-rtds fail
test-amd64-i386-xl-qemut-winxpsp3-vcpus1 pass
test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 pass
test-amd64-amd64-libvirt-vhd fail
test-armhf-armhf-xl-vhd fail
test-amd64-amd64-xl-qemut-winxpsp3 pass
test-amd64-i386-xl-qemut-winxpsp3 pass
test-amd64-amd64-xl-qemuu-winxpsp3 pass
test-amd64-i386-xl-qemuu-winxpsp3 pass
------------------------------------------------------------
sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images
Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs
Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master
Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary
Not pushing.
(No revision log; it would be 390 lines long.)
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [xen-unstable test] 78395: regressions - FAIL
2016-01-18 12:10 [xen-unstable test] 78395: regressions - FAIL osstest service owner
@ 2016-01-18 12:55 ` Jan Beulich
2016-01-18 14:39 ` [PATCH] x86/HVM: add padding to hvm_hw_cpu Roger Pau Monne
0 siblings, 1 reply; 16+ messages in thread
From: Jan Beulich @ 2016-01-18 12:55 UTC (permalink / raw)
To: Roger Pau Monne, xen-devel; +Cc: osstest-admin
>>> On 18.01.16 at 13:10, <osstest-admin@xenproject.org> wrote:
> test-amd64-i386-xl-qemut-win7-amd64 9 windows-install fail REGR. vs. 77892
> test-amd64-i386-xl-qemuu-win7-amd64 9 windows-install fail REGR. vs. 77892
Hmm, between all those XSM-related failures I didn't notice that
we've acquired an apparent regression here between 20c8f1a8a5
and 7167222b15 (based on this first failing in flight 78129). Oddly
enough the respective test-amd64-amd64-* tests don't exhibit
the issue, but none of the commits in between seem, at the first
glance, to affect 32-bit vs 64-bit Dom0 in different ways. Short
of anyone else having an idea, I guess we'll have to wait for a
bisect flight to complete.
Or wait - isn't the flag field introduction (commit a3b6844d3b)
incompatible, in that it fails to also add a 32-bit padding field?
Jan
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] x86/HVM: add padding to hvm_hw_cpu
2016-01-18 12:55 ` Jan Beulich
@ 2016-01-18 14:39 ` Roger Pau Monne
2016-01-18 14:47 ` Andrew Cooper
0 siblings, 1 reply; 16+ messages in thread
From: Roger Pau Monne @ 2016-01-18 14:39 UTC (permalink / raw)
To: xen-devel
Cc: Andrew Cooper, Ian Jackson, Ian Campbell, Jan Beulich,
Roger Pau Monne
So that the size of the structure is the same on 32 and 64bit.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
This should fix the issues seen on OSSTest when using a 32bit toolstack on
a 64bit hypervisor to create a Windows 7 HVM guest.
---
xen/include/public/arch-x86/hvm/save.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/xen/include/public/arch-x86/hvm/save.h b/xen/include/public/arch-x86/hvm/save.h
index b6b1bf8..6862720 100644
--- a/xen/include/public/arch-x86/hvm/save.h
+++ b/xen/include/public/arch-x86/hvm/save.h
@@ -163,6 +163,7 @@ struct hvm_hw_cpu {
#define _XEN_X86_FPU_INITIALISED 0
#define XEN_X86_FPU_INITIALISED (1U<<_XEN_X86_FPU_INITIALISED)
uint32_t flags;
+ uint32_t pad0;
};
struct hvm_hw_cpu_compat {
--
1.9.5 (Apple Git-50.3)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] x86/HVM: add padding to hvm_hw_cpu
2016-01-18 14:39 ` [PATCH] x86/HVM: add padding to hvm_hw_cpu Roger Pau Monne
@ 2016-01-18 14:47 ` Andrew Cooper
2016-01-18 15:09 ` [PATCH v2] x86/HVM: change the flags cpu context field to uint64_t Roger Pau Monne
2016-01-18 15:17 ` [PATCH] x86/HVM: add padding to hvm_hw_cpu Jan Beulich
0 siblings, 2 replies; 16+ messages in thread
From: Andrew Cooper @ 2016-01-18 14:47 UTC (permalink / raw)
To: Roger Pau Monne, xen-devel; +Cc: Ian Jackson, Ian Campbell, Jan Beulich
On 18/01/16 14:39, Roger Pau Monne wrote:
> So that the size of the structure is the same on 32 and 64bit.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> This should fix the issues seen on OSSTest when using a 32bit toolstack on
> a 64bit hypervisor to create a Windows 7 HVM guest.
> ---
> xen/include/public/arch-x86/hvm/save.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/xen/include/public/arch-x86/hvm/save.h b/xen/include/public/arch-x86/hvm/save.h
> index b6b1bf8..6862720 100644
> --- a/xen/include/public/arch-x86/hvm/save.h
> +++ b/xen/include/public/arch-x86/hvm/save.h
> @@ -163,6 +163,7 @@ struct hvm_hw_cpu {
> #define _XEN_X86_FPU_INITIALISED 0
> #define XEN_X86_FPU_INITIALISED (1U<<_XEN_X86_FPU_INITIALISED)
> uint32_t flags;
> + uint32_t pad0;
I would extend flags to uint64_t, so the existing ctxt.flags &
~XEN_X86_FPU_INITIALISED check for unused bits will cover all of them.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2] x86/HVM: change the flags cpu context field to uint64_t
2016-01-18 14:47 ` Andrew Cooper
@ 2016-01-18 15:09 ` Roger Pau Monne
2016-01-18 15:11 ` Andrew Cooper
2016-01-18 15:21 ` Jan Beulich
2016-01-18 15:17 ` [PATCH] x86/HVM: add padding to hvm_hw_cpu Jan Beulich
1 sibling, 2 replies; 16+ messages in thread
From: Roger Pau Monne @ 2016-01-18 15:09 UTC (permalink / raw)
To: xen-devel
Cc: Andrew Cooper, Ian Jackson, Ian Campbell, Jan Beulich,
Roger Pau Monne
So that the size of the structure is the same on 32 and 64bit.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
This should fix the issues seen on OSSTest when using a 32bit toolstack on
a 64bit hypervisor to create a Windows 7 HVM guest.
---
Changes since v1:
- Instead of adding padding, change the flags field to be a uint64_t.
---
xen/arch/x86/hvm/hvm.c | 2 +-
xen/include/public/arch-x86/hvm/save.h | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index a99edc2..1364d16 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2017,7 +2017,7 @@ static int hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
if ( (ctxt.flags & ~XEN_X86_FPU_INITIALISED) != 0 )
{
- gprintk(XENLOG_ERR, "bad flags value in CPU context: %#x\n",
+ gprintk(XENLOG_ERR, "bad flags value in CPU context: %#lx\n",
ctxt.flags);
return -EINVAL;
}
diff --git a/xen/include/public/arch-x86/hvm/save.h b/xen/include/public/arch-x86/hvm/save.h
index b6b1bf8..3fac45b 100644
--- a/xen/include/public/arch-x86/hvm/save.h
+++ b/xen/include/public/arch-x86/hvm/save.h
@@ -161,8 +161,8 @@ struct hvm_hw_cpu {
uint32_t error_code;
#define _XEN_X86_FPU_INITIALISED 0
-#define XEN_X86_FPU_INITIALISED (1U<<_XEN_X86_FPU_INITIALISED)
- uint32_t flags;
+#define XEN_X86_FPU_INITIALISED (1UL<<_XEN_X86_FPU_INITIALISED)
+ uint64_t flags;
};
struct hvm_hw_cpu_compat {
--
1.9.5 (Apple Git-50.3)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2] x86/HVM: change the flags cpu context field to uint64_t
2016-01-18 15:09 ` [PATCH v2] x86/HVM: change the flags cpu context field to uint64_t Roger Pau Monne
@ 2016-01-18 15:11 ` Andrew Cooper
2016-01-18 15:21 ` Jan Beulich
1 sibling, 0 replies; 16+ messages in thread
From: Andrew Cooper @ 2016-01-18 15:11 UTC (permalink / raw)
To: Roger Pau Monne, xen-devel; +Cc: Ian Jackson, Ian Campbell, Jan Beulich
On 18/01/16 15:09, Roger Pau Monne wrote:
> So that the size of the structure is the same on 32 and 64bit.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] x86/HVM: add padding to hvm_hw_cpu
2016-01-18 14:47 ` Andrew Cooper
2016-01-18 15:09 ` [PATCH v2] x86/HVM: change the flags cpu context field to uint64_t Roger Pau Monne
@ 2016-01-18 15:17 ` Jan Beulich
2016-01-18 16:25 ` [PATCH v3] x86/HVM: add padding to struct hvm_hw_cpu Roger Pau Monne
1 sibling, 1 reply; 16+ messages in thread
From: Jan Beulich @ 2016-01-18 15:17 UTC (permalink / raw)
To: Andrew Cooper, Roger Pau Monne, xen-devel; +Cc: Ian Jackson, Ian Campbell
>>> On 18.01.16 at 15:47, <andrew.cooper3@citrix.com> wrote:
>> index b6b1bf8..6862720 100644
>> --- a/xen/include/public/arch-x86/hvm/save.h
>> +++ b/xen/include/public/arch-x86/hvm/save.h
>> @@ -163,6 +163,7 @@ struct hvm_hw_cpu {
>> #define _XEN_X86_FPU_INITIALISED 0
>> #define XEN_X86_FPU_INITIALISED (1U<<_XEN_X86_FPU_INITIALISED)
>> uint32_t flags;
>> + uint32_t pad0;
>
> I would extend flags to uint64_t, so the existing ctxt.flags &
> ~XEN_X86_FPU_INITIALISED check for unused bits will cover all of them.
Actually I would have suggested against that (and demanded the
new field to be checked), but I see the new patch and your R-b
already came through, so I'm not going to demand a 3rd version.
Jan
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] x86/HVM: change the flags cpu context field to uint64_t
2016-01-18 15:09 ` [PATCH v2] x86/HVM: change the flags cpu context field to uint64_t Roger Pau Monne
2016-01-18 15:11 ` Andrew Cooper
@ 2016-01-18 15:21 ` Jan Beulich
2016-01-18 15:24 ` Andrew Cooper
1 sibling, 1 reply; 16+ messages in thread
From: Jan Beulich @ 2016-01-18 15:21 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: Andrew Cooper, Ian Jackson, Ian Campbell, xen-devel
>>> On 18.01.16 at 16:09, <roger.pau@citrix.com> wrote:
> --- a/xen/include/public/arch-x86/hvm/save.h
> +++ b/xen/include/public/arch-x86/hvm/save.h
> @@ -161,8 +161,8 @@ struct hvm_hw_cpu {
> uint32_t error_code;
>
> #define _XEN_X86_FPU_INITIALISED 0
> -#define XEN_X86_FPU_INITIALISED (1U<<_XEN_X86_FPU_INITIALISED)
> - uint32_t flags;
> +#define XEN_X86_FPU_INITIALISED (1UL<<_XEN_X86_FPU_INITIALISED)
> + uint64_t flags;
> };
How is the UL going to make this safe for a 32-bit consumer?
Makes me think that, other than just said in reply to v1, it'll
indeed be better to have a separate field (with a separate
zero-check)... The (undesirable imo) alternative being to use
1L instead.
Jan
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] x86/HVM: change the flags cpu context field to uint64_t
2016-01-18 15:21 ` Jan Beulich
@ 2016-01-18 15:24 ` Andrew Cooper
2016-01-18 15:38 ` Roger Pau Monné
0 siblings, 1 reply; 16+ messages in thread
From: Andrew Cooper @ 2016-01-18 15:24 UTC (permalink / raw)
To: Jan Beulich, Roger Pau Monne; +Cc: xen-devel, Ian Jackson, Ian Campbell
On 18/01/16 15:21, Jan Beulich wrote:
>>>> On 18.01.16 at 16:09, <roger.pau@citrix.com> wrote:
>> --- a/xen/include/public/arch-x86/hvm/save.h
>> +++ b/xen/include/public/arch-x86/hvm/save.h
>> @@ -161,8 +161,8 @@ struct hvm_hw_cpu {
>> uint32_t error_code;
>>
>> #define _XEN_X86_FPU_INITIALISED 0
>> -#define XEN_X86_FPU_INITIALISED (1U<<_XEN_X86_FPU_INITIALISED)
>> - uint32_t flags;
>> +#define XEN_X86_FPU_INITIALISED (1UL<<_XEN_X86_FPU_INITIALISED)
>> + uint64_t flags;
>> };
> How is the UL going to make this safe for a 32-bit consumer?
> Makes me think that, other than just said in reply to v1, it'll
> indeed be better to have a separate field (with a separate
> zero-check)... The (undesirable imo) alternative being to use
> 1L instead.
I am happy either way. My R-b stands.
~Andrew
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] x86/HVM: change the flags cpu context field to uint64_t
2016-01-18 15:24 ` Andrew Cooper
@ 2016-01-18 15:38 ` Roger Pau Monné
2016-01-18 16:24 ` Jan Beulich
0 siblings, 1 reply; 16+ messages in thread
From: Roger Pau Monné @ 2016-01-18 15:38 UTC (permalink / raw)
To: Andrew Cooper, Jan Beulich; +Cc: xen-devel, Ian Jackson, Ian Campbell
El 18/01/16 a les 16.24, Andrew Cooper ha escrit:
> On 18/01/16 15:21, Jan Beulich wrote:
>>>>> On 18.01.16 at 16:09, <roger.pau@citrix.com> wrote:
>>> --- a/xen/include/public/arch-x86/hvm/save.h
>>> +++ b/xen/include/public/arch-x86/hvm/save.h
>>> @@ -161,8 +161,8 @@ struct hvm_hw_cpu {
>>> uint32_t error_code;
>>>
>>> #define _XEN_X86_FPU_INITIALISED 0
>>> -#define XEN_X86_FPU_INITIALISED (1U<<_XEN_X86_FPU_INITIALISED)
>>> - uint32_t flags;
>>> +#define XEN_X86_FPU_INITIALISED (1UL<<_XEN_X86_FPU_INITIALISED)
>>> + uint64_t flags;
>>> };
>> How is the UL going to make this safe for a 32-bit consumer?
>> Makes me think that, other than just said in reply to v1, it'll
>> indeed be better to have a separate field (with a separate
>> zero-check)... The (undesirable imo) alternative being to use
>> 1L instead.
>
> I am happy either way. My R-b stands.
What about using ULL or simply casting to uint64_t?
Roger.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] x86/HVM: change the flags cpu context field to uint64_t
2016-01-18 15:38 ` Roger Pau Monné
@ 2016-01-18 16:24 ` Jan Beulich
2016-01-18 16:33 ` Roger Pau Monné
0 siblings, 1 reply; 16+ messages in thread
From: Jan Beulich @ 2016-01-18 16:24 UTC (permalink / raw)
To: Roger Pau Monné; +Cc: Andrew Cooper, Ian Jackson, Ian Campbell, xen-devel
>>> On 18.01.16 at 16:38, <roger.pau@citrix.com> wrote:
> El 18/01/16 a les 16.24, Andrew Cooper ha escrit:
>> On 18/01/16 15:21, Jan Beulich wrote:
>>>>>> On 18.01.16 at 16:09, <roger.pau@citrix.com> wrote:
>>>> --- a/xen/include/public/arch-x86/hvm/save.h
>>>> +++ b/xen/include/public/arch-x86/hvm/save.h
>>>> @@ -161,8 +161,8 @@ struct hvm_hw_cpu {
>>>> uint32_t error_code;
>>>>
>>>> #define _XEN_X86_FPU_INITIALISED 0
>>>> -#define XEN_X86_FPU_INITIALISED (1U<<_XEN_X86_FPU_INITIALISED)
>>>> - uint32_t flags;
>>>> +#define XEN_X86_FPU_INITIALISED (1UL<<_XEN_X86_FPU_INITIALISED)
>>>> + uint64_t flags;
>>>> };
>>> How is the UL going to make this safe for a 32-bit consumer?
>>> Makes me think that, other than just said in reply to v1, it'll
>>> indeed be better to have a separate field (with a separate
>>> zero-check)... The (undesirable imo) alternative being to use
>>> 1L instead.
>>
>> I am happy either way. My R-b stands.
>
> What about using ULL or simply casting to uint64_t?
ULL might not be supported by pre-C99 compilers. Casting to
uint64_t is, well, ugly. The flags field really has no business
being wider then 32 bits.
Jan
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3] x86/HVM: add padding to struct hvm_hw_cpu
2016-01-18 15:17 ` [PATCH] x86/HVM: add padding to hvm_hw_cpu Jan Beulich
@ 2016-01-18 16:25 ` Roger Pau Monne
2016-01-19 15:13 ` Jan Beulich
0 siblings, 1 reply; 16+ messages in thread
From: Roger Pau Monne @ 2016-01-18 16:25 UTC (permalink / raw)
To: xen-devel
Cc: Andrew Cooper, Ian Jackson, Ian Campbell, Jan Beulich,
Roger Pau Monne
So that the size of the structure is the same on 32 and 64bit.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
This should fix the issues seen on OSSTest when using a 32bit toolstack on
a 64bit hypervisor to create a Windows 7 HVM guest.
---
Chnges since v2:
- Fall back to adding a padding field and properly checking for it to be 0.
Changes since v1:
- Instead of adding padding, change the flags field to be a uint64_t.
---
xen/arch/x86/hvm/hvm.c | 3 +++
xen/include/public/arch-x86/hvm/save.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index a99edc2..cc5d14b 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1990,6 +1990,9 @@ static int hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
if ( hvm_load_entry_zeroextend(CPU, h, &ctxt) != 0 )
return -EINVAL;
+ if ( ctxt.pad0 != 0 )
+ return -EINVAL;
+
/* Sanity check some control registers. */
if ( (ctxt.cr0 & HVM_CR0_GUEST_RESERVED_BITS) ||
!(ctxt.cr0 & X86_CR0_ET) ||
diff --git a/xen/include/public/arch-x86/hvm/save.h b/xen/include/public/arch-x86/hvm/save.h
index b6b1bf8..6862720 100644
--- a/xen/include/public/arch-x86/hvm/save.h
+++ b/xen/include/public/arch-x86/hvm/save.h
@@ -163,6 +163,7 @@ struct hvm_hw_cpu {
#define _XEN_X86_FPU_INITIALISED 0
#define XEN_X86_FPU_INITIALISED (1U<<_XEN_X86_FPU_INITIALISED)
uint32_t flags;
+ uint32_t pad0;
};
struct hvm_hw_cpu_compat {
--
1.9.5 (Apple Git-50.3)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2] x86/HVM: change the flags cpu context field to uint64_t
2016-01-18 16:24 ` Jan Beulich
@ 2016-01-18 16:33 ` Roger Pau Monné
2016-01-18 16:43 ` Jan Beulich
0 siblings, 1 reply; 16+ messages in thread
From: Roger Pau Monné @ 2016-01-18 16:33 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Ian Jackson, Ian Campbell, xen-devel
El 18/01/16 a les 17.24, Jan Beulich ha escrit:
>>>> On 18.01.16 at 16:38, <roger.pau@citrix.com> wrote:
>> El 18/01/16 a les 16.24, Andrew Cooper ha escrit:
>>> On 18/01/16 15:21, Jan Beulich wrote:
>>>>>>> On 18.01.16 at 16:09, <roger.pau@citrix.com> wrote:
>>>>> --- a/xen/include/public/arch-x86/hvm/save.h
>>>>> +++ b/xen/include/public/arch-x86/hvm/save.h
>>>>> @@ -161,8 +161,8 @@ struct hvm_hw_cpu {
>>>>> uint32_t error_code;
>>>>>
>>>>> #define _XEN_X86_FPU_INITIALISED 0
>>>>> -#define XEN_X86_FPU_INITIALISED (1U<<_XEN_X86_FPU_INITIALISED)
>>>>> - uint32_t flags;
>>>>> +#define XEN_X86_FPU_INITIALISED (1UL<<_XEN_X86_FPU_INITIALISED)
>>>>> + uint64_t flags;
>>>>> };
>>>> How is the UL going to make this safe for a 32-bit consumer?
>>>> Makes me think that, other than just said in reply to v1, it'll
>>>> indeed be better to have a separate field (with a separate
>>>> zero-check)... The (undesirable imo) alternative being to use
>>>> 1L instead.
>>>
>>> I am happy either way. My R-b stands.
>>
>> What about using ULL or simply casting to uint64_t?
>
> ULL might not be supported by pre-C99 compilers. Casting to
> uint64_t is, well, ugly. The flags field really has no business
> being wider then 32 bits.
Right, although Xen uses gnu99 this is a public header.
You should see a v3 somewhere with a proper check for the padding filed.
While doing this I've also realised that the padding fields in the other
structs in the same file don't seem to be checked at all.
Roger.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] x86/HVM: change the flags cpu context field to uint64_t
2016-01-18 16:33 ` Roger Pau Monné
@ 2016-01-18 16:43 ` Jan Beulich
0 siblings, 0 replies; 16+ messages in thread
From: Jan Beulich @ 2016-01-18 16:43 UTC (permalink / raw)
To: Roger Pau Monné; +Cc: Andrew Cooper, Ian Jackson, Ian Campbell, xen-devel
>>> On 18.01.16 at 17:33, <roger.pau@citrix.com> wrote:
> El 18/01/16 a les 17.24, Jan Beulich ha escrit:
>> ULL might not be supported by pre-C99 compilers. Casting to
>> uint64_t is, well, ugly. The flags field really has no business
>> being wider then 32 bits.
>
> Right, although Xen uses gnu99 this is a public header.
>
> You should see a v3 somewhere with a proper check for the padding filed.
Thanks.
> While doing this I've also realised that the padding fields in the other
> structs in the same file don't seem to be checked at all.
For ones which already went out in a release we may not be able to
do anything retroactively.
Jan
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3] x86/HVM: add padding to struct hvm_hw_cpu
2016-01-18 16:25 ` [PATCH v3] x86/HVM: add padding to struct hvm_hw_cpu Roger Pau Monne
@ 2016-01-19 15:13 ` Jan Beulich
2016-01-19 15:21 ` Roger Pau Monné
0 siblings, 1 reply; 16+ messages in thread
From: Jan Beulich @ 2016-01-19 15:13 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: Andrew Cooper, Ian Jackson, Ian Campbell, xen-devel
>>> On 18.01.16 at 17:25, <roger.pau@citrix.com> wrote:
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -1990,6 +1990,9 @@ static int hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
> if ( hvm_load_entry_zeroextend(CPU, h, &ctxt) != 0 )
> return -EINVAL;
>
> + if ( ctxt.pad0 != 0 )
> + return -EINVAL;
Right after I had committed and pushed the patch it occurred to
me that this seems to be missing a save side counterpart, which
would constitute both an information leak and a functional bug.
Would you please take another look?
Jan
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3] x86/HVM: add padding to struct hvm_hw_cpu
2016-01-19 15:13 ` Jan Beulich
@ 2016-01-19 15:21 ` Roger Pau Monné
0 siblings, 0 replies; 16+ messages in thread
From: Roger Pau Monné @ 2016-01-19 15:21 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Ian Jackson, Ian Campbell, xen-devel
El 19/01/16 a les 16.13, Jan Beulich ha escrit:
>>>> On 18.01.16 at 17:25, <roger.pau@citrix.com> wrote:
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -1990,6 +1990,9 @@ static int hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
>> if ( hvm_load_entry_zeroextend(CPU, h, &ctxt) != 0 )
>> return -EINVAL;
>>
>> + if ( ctxt.pad0 != 0 )
>> + return -EINVAL;
>
> Right after I had committed and pushed the patch it occurred to
> me that this seems to be missing a save side counterpart, which
> would constitute both an information leak and a functional bug.
> Would you please take another look?
Sure, thanks for realising! Now that you make me look at it
hvm_save_cpu_ctxt should zero hvm_hw_cpu on each iteration, IMHO the
current code is asking for trouble. I will send a patch ASAP.
Roger.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2016-01-19 15:21 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-18 12:10 [xen-unstable test] 78395: regressions - FAIL osstest service owner
2016-01-18 12:55 ` Jan Beulich
2016-01-18 14:39 ` [PATCH] x86/HVM: add padding to hvm_hw_cpu Roger Pau Monne
2016-01-18 14:47 ` Andrew Cooper
2016-01-18 15:09 ` [PATCH v2] x86/HVM: change the flags cpu context field to uint64_t Roger Pau Monne
2016-01-18 15:11 ` Andrew Cooper
2016-01-18 15:21 ` Jan Beulich
2016-01-18 15:24 ` Andrew Cooper
2016-01-18 15:38 ` Roger Pau Monné
2016-01-18 16:24 ` Jan Beulich
2016-01-18 16:33 ` Roger Pau Monné
2016-01-18 16:43 ` Jan Beulich
2016-01-18 15:17 ` [PATCH] x86/HVM: add padding to hvm_hw_cpu Jan Beulich
2016-01-18 16:25 ` [PATCH v3] x86/HVM: add padding to struct hvm_hw_cpu Roger Pau Monne
2016-01-19 15:13 ` Jan Beulich
2016-01-19 15:21 ` Roger Pau Monné
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).