[Xen-devel] [xen-4.5-testing test] 103755: trouble: broken/fail/pass

[osstest service owner]

flight 103755 xen-4.5-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/103755/

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemut-debianhvm-amd64 3 host-install(3) broken REGR. vs. 
103398
 test-amd64-amd64-amd64-pvgrub  3 host-install(3)   broken REGR. vs. 103398
 test-amd64-amd64-libvirt  3 host-install(3)broken REGR. vs. 103398
 test-amd64-i386-xl3 host-install(3)broken REGR. vs. 103398
 test-amd64-i386-xl-qemuu-debianhvm-amd64 3 host-install(3) broken REGR. vs. 
103398
 test-amd64-amd64-xl-multivcpu  3 host-install(3)   broken REGR. vs. 103398
 test-amd64-i386-qemut-rhel6hvm-amd  3 host-install(3)  broken REGR. vs. 103398

Regressions which are regarded as allowable (not blocking):
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 103316
 test-amd64-amd64-xl-rtds  6 xen-boot fail  like 103398
 test-xtf-amd64-amd64-2   52 leak-check/check fail  like 103398
 test-xtf-amd64-amd64-3   52 leak-check/check fail  like 103398
 test-xtf-amd64-amd64-1   52 leak-check/check fail  like 103398
 test-armhf-armhf-libvirt 13 saverestore-support-checkfail  like 103398
 test-xtf-amd64-amd64-4   52 leak-check/check fail  like 103398
 test-xtf-amd64-amd64-5   52 leak-check/check fail  like 103398
 test-armhf-armhf-xl-rtds 11 guest-start  fail  like 103398
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 103398
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stopfail like 103398
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 103398
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 15 guest-localmigrate/x10 fail like 
103398

Tests which did not succeed, but are not blocking:
 test-xtf-amd64-amd64-2   18 xtf/test-hvm32-cpuid-faulting fail  never pass
 test-xtf-amd64-amd64-2 29 xtf/test-hvm32pae-cpuid-faulting fail never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-xtf-amd64-amd64-2 35 xtf/test-hvm32pse-cpuid-faulting fail never pass
 test-xtf-amd64-amd64-2   39 xtf/test-hvm64-cpuid-faulting fail  never pass
 test-xtf-amd64-amd64-4   18 xtf/test-hvm32-cpuid-faulting fail  never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-xtf-amd64-amd64-4 29 xtf/test-hvm32pae-cpuid-faulting fail never pass
 test-xtf-amd64-amd64-5   18 xtf/test-hvm32-cpuid-faulting fail  never pass
 test-xtf-amd64-amd64-1   18 xtf/test-hvm32-cpuid-faulting fail  never pass
 test-xtf-amd64-amd64-4 35 xtf/test-hvm32pse-cpuid-faulting fail never pass
 test-xtf-amd64-amd64-4   39 xtf/test-hvm64-cpuid-faulting fail  never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-xtf-amd64-amd64-2   51 xtf/test-hvm64-xsa-195   fail   never pass
 test-xtf-amd64-amd64-5 29 xtf/test-hvm32pae-cpuid-faulting fail never pass
 test-xtf-amd64-amd64-5 35 xtf/test-hvm32pse-cpuid-faulting fail never pass
 test-xtf-amd64-amd64-1 29 xtf/test-hvm32pae-cpuid-faulting fail never pass
 test-xtf-amd64-amd64-5   39 xtf/test-hvm64-cpuid-faulting fail  never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-xtf-amd64-amd64-1 35 xtf/test-hvm32pse-cpuid-faulting fail never pass
 test-xtf-amd64-amd64-1   39 xtf/test-hvm64-cpuid-faulting fail  never pass
 test-xtf-amd64-amd64-3   51 xtf/test-hvm64-xsa-195   fail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-xtf-amd64-amd64-1   51 xtf/test-hvm64-xsa-195   fail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-xtf-amd64-amd64-4   51 xtf/test-hvm64-xsa-195   fail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-xtf-amd64-amd64-5   51 xtf/test-hvm64-xsa-195   fail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 10 guest-start  fail   never pass
 test-

Re: [Xen-devel] [PATCH 1/2] x86/hvm: Don't emulate all instructions hitting the #UD intercept

[Jan Beulich]

>>> On 19.12.16 at 17:37,  wrote:
> Having the instruction emulator fill in all #UDs when using FEP is unhelpful
> when trying to test emulation behaviour against hardware.
> 
> Restrict emulation from the #UD intercept to the cross-vendor case, and when a
> postive Forced Emulation Prefix has been identified.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Jan Beulich 


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH] x86emul: don't unconditionally clear segment bases upon null selector loads

[Jan Beulich]

AMD explicitly documents that namely FS and GS don't have their bases
cleared in that case, and I see no reason why guests may not rely on
that behavior. To facilitate this a new input field (the CPU vendor) is
being added.

Signed-off-by: Jan Beulich 

--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -119,6 +119,7 @@ int main(int argc, char **argv)
 
 ctxt.regs = ®s;
 ctxt.force_writeback = 0;
+ctxt.vendor= X86_VENDOR_UNKNOWN;
 ctxt.addr_size = 8 * sizeof(void *);
 ctxt.sp_size   = 8 * sizeof(void *);
 
--- a/tools/tests/x86_emulator/x86_emulate.h
+++ b/tools/tests/x86_emulator/x86_emulate.h
@@ -38,6 +38,11 @@
 
 #define is_canonical_address(x) (((int64_t)(x) >> 47) == ((int64_t)(x) >> 63))
 
+/* There's no strict need for these to be in sync with processor.h. */
+#define X86_VENDOR_INTEL   0
+#define X86_VENDOR_AMD 2
+#define X86_VENDOR_UNKNOWN 0xff
+
 #define MMAP_SZ 16384
 bool emul_test_make_stack_executable(void);
 
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -1897,6 +1897,7 @@ void hvm_emulate_init_once(
 hvmemul_get_seg_reg(x86_seg_ss, hvmemul_ctxt);
 
 hvmemul_ctxt->ctxt.regs = regs;
+hvmemul_ctxt->ctxt.vendor = current->domain->arch.x86_vendor;
 hvmemul_ctxt->ctxt.force_writeback = true;
 
 if ( cpu_has_vmx )
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5358,6 +5358,7 @@ int ptwr_do_page_fault(struct vcpu *v, u
 struct ptwr_emulate_ctxt ptwr_ctxt = {
 .ctxt = {
 .regs = regs,
+.vendor = d->arch.x86_vendor,
 .addr_size = is_pv_32bit_domain(d) ? 32 : BITS_PER_LONG,
 .sp_size   = is_pv_32bit_domain(d) ? 32 : BITS_PER_LONG,
 .swint_emulate = x86_swint_emulate_none,
@@ -5513,6 +5514,7 @@ int mmio_ro_do_page_fault(struct vcpu *v
 struct mmio_ro_emulate_ctxt mmio_ro_ctxt = { .cr2 = addr };
 struct x86_emulate_ctxt ctxt = {
 .regs = regs,
+.vendor = v->domain->arch.x86_vendor,
 .addr_size = addr_size,
 .sp_size = addr_size,
 .swint_emulate = x86_swint_emulate_none,
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -330,6 +330,7 @@ const struct x86_emulate_ops *shadow_ini
 memset(sh_ctxt, 0, sizeof(*sh_ctxt));
 
 sh_ctxt->ctxt.regs = regs;
+sh_ctxt->ctxt.vendor = v->domain->arch.x86_vendor;
 sh_ctxt->ctxt.swint_emulate = x86_swint_emulate_none;
 
 /* Segment cache initialisation. Primed with CS. */
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -3352,7 +3352,10 @@ static int emulate_privileged_op(struct
 {
 struct vcpu *curr = current;
 struct domain *currd = curr->domain;
-struct priv_op_ctxt ctxt = { .ctxt.regs = regs };
+struct priv_op_ctxt ctxt = {
+.ctxt.regs = regs,
+.ctxt.vendor = currd->arch.x86_vendor,
+};
 int rc;
 unsigned int eflags, ar;
 
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1395,7 +1395,11 @@ protmode_load_seg(
 case x86_seg_tr:
 goto raise_exn;
 }
-memset(sreg, 0, sizeof(*sreg));
+if ( ctxt->vendor != X86_VENDOR_AMD || !ops->read_segment ||
+ ops->read_segment(seg, sreg, ctxt) != X86EMUL_OKAY )
+memset(sreg, 0, sizeof(*sreg));
+else
+sreg->attr.bytes = 0;
 sreg->sel = sel;
 return X86EMUL_OKAY;
 }
--- a/xen/arch/x86/x86_emulate/x86_emulate.h
+++ b/xen/arch/x86/x86_emulate/x86_emulate.h
@@ -462,6 +462,9 @@ struct x86_emulate_ctxt
 /* Software event injection support. */
 enum x86_swint_emulation swint_emulate;
 
+/* CPU vendor (X86_VENDOR_UNKNOWN for "don't care") */
+unsigned char vendor;
+
 /* Set this if writes may have side effects. */
 bool force_writeback;
 



x86emul: don't unconditionally clear segment bases upon null selector loads

AMD explicitly documents that namely FS and GS don't have their bases
cleared in that case, and I see no reason why guests may not rely on
that behavior. To facilitate this a new input field (the CPU vendor) is
being added.

Signed-off-by: Jan Beulich 

--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -119,6 +119,7 @@ int main(int argc, char **argv)
 
 ctxt.regs = ®s;
 ctxt.force_writeback = 0;
+ctxt.vendor= X86_VENDOR_UNKNOWN;
 ctxt.addr_size = 8 * sizeof(void *);
 ctxt.sp_size   = 8 * sizeof(void *);
 
--- a/tools/tests/x86_emulator/x86_emulate.h
+++ b/tools/tests/x86_emulator/x86_emulate.h
@@ -38,6 +38,11 @@
 
 #define is_canonical_address(x) (((int64_t)(x) >> 47) == ((int64_t)(x) >> 63))
 
+/* There's no strict need for these to be in sync with processor.h. */
+#define X86_VENDOR_INTEL   0
+#define X86_VENDOR_AMD 2
+#define X86_VENDOR_UNKNOWN 0xff
+
 #define MMAP_SZ 16384
 bool emul_test_make_stack_executable(void);
 
--- a/xen/a

Re: [Xen-devel] [PATCH 2/2] x86/emul: Bugfixes to SYSCALL emulation

[Jan Beulich]

>>> On 19.12.16 at 17:37,  wrote:
> Introduce vendor_is() to allow emulation to have vendor-specific
> behaviour.  Adjust the SYSCALL behaviour on Intel to raise #UD when
> executed outside of 64bit mode.

I'd rather not see us go this route. I've been carrying a patch
making the vendor an input (not submitted so far due to other
at least contextual prereqs missing when I last check, but sent
out just a minute ago). What do you think?

> in_longmode() has different return semantics from rc, so use a separate
> integer for the purpose.

The ugliness of the additional #ifdef doesn't seem to warrant
this. What I've been thinking the other day though is: Why
don't we put the whole SYSCALL emulation into a __XEN__
conditional (implying __x86_64__, i.e. allowing the inner ones
to be removed)? That would likely also limit the impact of the
pending register name changes which I hope to be able to
submit soon (once enough prereqs have gone in).

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] x86/apicv: fix RTC periodic timer and apicv issue

[Jan Beulich]

>>> On 20.12.16 at 06:54,  wrote:
> On December 20, 2016 1:37 PM, Tian, Kevin wrote:
>>> From: Xuquan (Quan Xu) [mailto:xuqu...@huawei.com]
>>> Sent: Friday, December 16, 2016 5:40 PM
>>>
>>> From 89fffdd6b563b2723e24d17231715bb8c9f24f90 Mon Sep 17 00:00:00
>>2001
>>> From: Quan Xu 
>>> Date: Fri, 16 Dec 2016 17:24:01 +0800
>>> Subject: [PATCH v3] x86/apicv: fix RTC periodic timer and apicv issue
>>>
>>> When Xen apicv is enabled, wall clock time is faster on Windows7-32
>>> guest with high payload (with 2vCPU, captured from xentrace, in high
>>> payload, the count of IPI interrupt increases rapidly between these
>>> vCPUs).
>>>
>>> If IPI intrrupt (vector 0xe1) and periodic timer interrupt (vector
>>> 0xd1) are both pending (index of bit set in vIRR), unfortunately, the
>>> IPI intrrupt is high priority than periodic timer interrupt. Xen
>>> updates IPI interrupt bit set in vIRR to guest interrupt status (RVI)
>>> as a high priority and apicv (Virtual-Interrupt Delivery) delivers IPI
>>> interrupt within VMX non-root operation without a VM-Exit. Within VMX
>>> non-root operation, if periodic timer interrupt index of bit is set in
>>> vIRR and highest, the apicv delivers periodic timer interrupt within
>>> VMX non-root operation as well.
>>>
>>> But in current code, if Xen doesn't update periodic timer interrupt
>>> bit set in vIRR to guest interrupt status (RVI) directly, Xen is not
>>> aware of this case to decrease the count (pending_intr_nr) of pending
>>> periodic timer interrupt, then Xen will deliver a periodic timer interrupt
>>again.
>>>
>>> And that we update periodic timer interrupt in every VM-entry, there
>>> is a chance that already-injected instance (before EOI-induced exit
>>> happens) will incur another pending IRR setting if there is a VM-exit
>>> happens between virtual interrupt injection (vIRR->0, vISR->1) and
>>> EOI-induced exit (vISR->0), since pt_intr_post hasn't been invoked
>>> yet, then the guest receives more periodic timer interrupt.
>>>
>>> So we set eoi_exit_bitmap for intack.vector when it's higher than
>>> pending periodic time interrupts. This way we can guarantee there's
>>> always a chance to post periodic time interrupts when periodic time
>>> interrupts becomes the highest one.
>>>
>>> Signed-off-by: Quan Xu 
>>
>>I suppose you've verified this new version, but still would like get your
>>explicit confirmation - did you still see time accuracy issue in your side?
>>Have you tried other guest OS types other than Win7-32?
>>
> 
> I only verified it on win7-32 guest..
> I will continue to verify it on other windows guest (I think windows is 
> enough, right?)

No, I don't think Windows alone is sufficient for verification. People
run all kinds of OSes as HVM guests, and your change should not
negatively impact them. At the very least you want to also try Linux.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] x86/apicv: fix RTC periodic timer and apicv issue

[Jan Beulich]

>>> On 20.12.16 at 06:37,  wrote:
>>  From: Xuquan (Quan Xu) [mailto:xuqu...@huawei.com]
>> Sent: Friday, December 16, 2016 5:40 PM
>> -if (pt_vector != -1)
>> -vmx_set_eoi_exit_bitmap(v, pt_vector);
>> +if ( pt_vector != -1 ) {
>> +if ( intack.vector > pt_vector )
>> +vmx_set_eoi_exit_bitmap(v, intack.vector);
>> +else
>> +vmx_set_eoi_exit_bitmap(v, pt_vector);
>> +}
> 
> Above can be simplified as one line change:
>   if ( pt_vector != -1 )
>   vmx_set_eoi_exit_bitmap(v, intack.vector);

Hmm, I don't understand. Did you mean to use max() here? Or
else how is this an equivalent of the originally proposed code?

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] x86/apicv: fix RTC periodic timer and apicv issue

[Tian, Kevin]

> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: Tuesday, December 20, 2016 4:35 PM
> 
> >>> On 20.12.16 at 06:37,  wrote:
> >>  From: Xuquan (Quan Xu) [mailto:xuqu...@huawei.com]
> >> Sent: Friday, December 16, 2016 5:40 PM
> >> -if (pt_vector != -1)
> >> -vmx_set_eoi_exit_bitmap(v, pt_vector);
> >> +if ( pt_vector != -1 ) {
> >> +if ( intack.vector > pt_vector )
> >> +vmx_set_eoi_exit_bitmap(v, intack.vector);
> >> +else
> >> +vmx_set_eoi_exit_bitmap(v, pt_vector);
> >> +}
> >
> > Above can be simplified as one line change:
> > if ( pt_vector != -1 )
> > vmx_set_eoi_exit_bitmap(v, intack.vector);
> 
> Hmm, I don't understand. Did you mean to use max() here? Or
> else how is this an equivalent of the originally proposed code?
> 

Original code is not 100% correct. The purpose is to set EOI exit
bitmap for any vector which may block injection of pt_vector - 
give chance to recognize pt_vector in future intack and then do pt 
intr post. The simplified code achieves this effect same as original
code if intack.vector >= vector. I cannot come up a case why
intack.vector might be smaller than vector. If this case happens,
we still need enable exit bitmap for intack.vector instead of
pt_vector for said purpose while original code did it wrong.

Thanks
Kevin

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] x86/apicv: fix RTC periodic timer and apicv issue

[Jan Beulich]

>>> On 20.12.16 at 09:53,  wrote:
>>  From: Jan Beulich [mailto:jbeul...@suse.com]
>> Sent: Tuesday, December 20, 2016 4:35 PM
>> 
>> >>> On 20.12.16 at 06:37,  wrote:
>> >>  From: Xuquan (Quan Xu) [mailto:xuqu...@huawei.com]
>> >> Sent: Friday, December 16, 2016 5:40 PM
>> >> -if (pt_vector != -1)
>> >> -vmx_set_eoi_exit_bitmap(v, pt_vector);
>> >> +if ( pt_vector != -1 ) {
>> >> +if ( intack.vector > pt_vector )
>> >> +vmx_set_eoi_exit_bitmap(v, intack.vector);
>> >> +else
>> >> +vmx_set_eoi_exit_bitmap(v, pt_vector);
>> >> +}
>> >
>> > Above can be simplified as one line change:
>> >if ( pt_vector != -1 )
>> >vmx_set_eoi_exit_bitmap(v, intack.vector);
>> 
>> Hmm, I don't understand. Did you mean to use max() here? Or
>> else how is this an equivalent of the originally proposed code?
>> 
> 
> Original code is not 100% correct. The purpose is to set EOI exit
> bitmap for any vector which may block injection of pt_vector - 
> give chance to recognize pt_vector in future intack and then do pt 
> intr post. The simplified code achieves this effect same as original
> code if intack.vector >= vector. I cannot come up a case why
> intack.vector might be smaller than vector. If this case happens,
> we still need enable exit bitmap for intack.vector instead of
> pt_vector for said purpose while original code did it wrong.

Ah, okay. Thanks for explaining this to me.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] Ping: [PATCH 0/3] x86emul: misc adjustments

[Jan Beulich]

>>> On 12.12.16 at 10:38,  wrote:
> These patches are grouped together merely because of contextual
> dependencies.
> 
> 1: correct EFLAGS.TF handling
> 2: conditionally clear BNDn for branches
> 3: some REX related polishing
> 
> Signed-off-by: Jan Beulich 

Any word on these? I realize patch 1 will need to be re-based over
yesterday's XSA-204 patch, and as mentioned elsewhere may also
want to suppress setting of retire.sti when TF=1 and BTF=0, but
before sending a v2 of that I'd prefer knowing whether further
adjustments may be needed.

Thanks, Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] xen: sched: removal of redundant check in Credit

[Praveen Kumar]

Hi Dario,

I tried with 'git am' to apply the patch after downloading the mbox file,
that worked fine. Do let me know if that is ok.

Regards,

~Praveen.

On Sat, Dec 17, 2016 at 1:44 PM, Praveen Kumar 
wrote:

> Hi,
>
> On Sat, Dec 17, 2016 at 7:16 AM, Dario Faggioli  > wrote:
>
>> On Sat, 2016-12-17 at 00:53 +0530, Praveen Kumar wrote:
>> > The patch gets rid of a redundant check in csched_vcpu_acct. In fact,
>> > the function is only called from csched_tick, which already checks
>> > that current is not the idle vcpu. The patch also adds an ASSERT to
>> > the same effect, in order to make assumption ( i.e., no calling this
>> > on idle vcpus) even more clear and as a guard for future mis-use.
>> >
>> > Signed-off-by: Praveen Kumar 
>> > Acked-by: Dario Faggioli 
>> >
>> Better than before. But still, if I:
>>  - save this mail as mbox
>>  - try to import it in git
>> it fails.
>>
>> OTOH, if I:
>>  - save this mail as mbox
>>  - run dos2unix on the mbox file
>>  - try to import it in git
>> it works!
>>
>> Which, since you're now using git-send-email, makes me think it may be
>> your editor/OS which is at fault and inserts spurious stuff at line
>> breaks (the classic CR vs. CR+LF thing).
>>
>> What editor on what OS are you using?
>>
>
> I am using SLED machine and vim editor. I tried checking with vimdiff over
> the patch generated and mbox file ( create by evolution ), I found spurious
> stuff (^M) as mentioned added. After dos2unix, the mbox file was same to
> that of the patch sent.
>
>>
>> Another possibility is that there is something wrong here at my hand.
>> George, do you have (similar) issues when trying to apply this patch?
>>
>> Regards,
>> Dario
>> --
>> <> (Raistlin Majere)
>> -
>> Dario Faggioli, Ph.D, http://about.me/dario.faggioli
>> Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
>
>
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xsm: allow relevant permission during migrate and gpu-passthrough.

[Anshul Makkar]

On 20/12/2016 04:03, Doug Goldstein wrote:

On 12/19/16 10:02 AM, Doug Goldstein wrote:

On 12/14/16 3:09 PM, Daniel De Graaf wrote:

On 12/12/2016 09:00 AM, Anshul Makkar wrote:

During guest migrate allow permission to prevent
spurious page faults.
Prevents these errors:
d73: Non-privileged (73) attempt to map I/O space 

avc: denied  { set_misc_info } for domid=0 target=11
scontext=system_u:system_r:dom0_t
tcontext=system_u:system_r:domU_t tclass=domain

GPU passthrough for hvm guest:
avc:  denied  { send_irq } for domid=0 target=10
scontext=system_u:system_r:dom0_t
tcontext=system_u:system_r:domU_t tclass=hvm

Signed-off-by: Anshul Makkar 


Acked-by: Daniel De Graaf 



Daniel,

Should this be backported to 4.8?



FWIW, Daniel's email is bouncing. Anshul, do you want to test/confirm?



Doug, yes, will backport and test.

Anshul

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] x86/apicv: fix RTC periodic timer and apicv issue

[Xuquan (Quan Xu)]

On December 20, 2016 4:32 PM, Jan Beulich wrote:
 On 20.12.16 at 06:54,  wrote:
>> On December 20, 2016 1:37 PM, Tian, Kevin wrote:
 From: Xuquan (Quan Xu) [mailto:xuqu...@huawei.com]
 Sent: Friday, December 16, 2016 5:40 PM

 From 89fffdd6b563b2723e24d17231715bb8c9f24f90 Mon Sep 17
>00:00:00
>>>2001
 From: Quan Xu 
 Date: Fri, 16 Dec 2016 17:24:01 +0800
 Subject: [PATCH v3] x86/apicv: fix RTC periodic timer and apicv
 issue

 When Xen apicv is enabled, wall clock time is faster on Windows7-32
 guest with high payload (with 2vCPU, captured from xentrace, in high
 payload, the count of IPI interrupt increases rapidly between these
 vCPUs).

 If IPI intrrupt (vector 0xe1) and periodic timer interrupt (vector
 0xd1) are both pending (index of bit set in vIRR), unfortunately,
 the IPI intrrupt is high priority than periodic timer interrupt. Xen
 updates IPI interrupt bit set in vIRR to guest interrupt status
 (RVI) as a high priority and apicv (Virtual-Interrupt Delivery)
 delivers IPI interrupt within VMX non-root operation without a
 VM-Exit. Within VMX non-root operation, if periodic timer interrupt
 index of bit is set in vIRR and highest, the apicv delivers periodic
 timer interrupt within VMX non-root operation as well.

 But in current code, if Xen doesn't update periodic timer interrupt
 bit set in vIRR to guest interrupt status (RVI) directly, Xen is not
 aware of this case to decrease the count (pending_intr_nr) of
 pending periodic timer interrupt, then Xen will deliver a periodic
 timer interrupt
>>>again.

 And that we update periodic timer interrupt in every VM-entry, there
 is a chance that already-injected instance (before EOI-induced exit
 happens) will incur another pending IRR setting if there is a
 VM-exit happens between virtual interrupt injection (vIRR->0,
 vISR->1) and EOI-induced exit (vISR->0), since pt_intr_post hasn't
 been invoked yet, then the guest receives more periodic timer
>interrupt.

 So we set eoi_exit_bitmap for intack.vector when it's higher than
 pending periodic time interrupts. This way we can guarantee there's
 always a chance to post periodic time interrupts when periodic time
 interrupts becomes the highest one.

 Signed-off-by: Quan Xu 
>>>
>>>I suppose you've verified this new version, but still would like get
>>>your explicit confirmation - did you still see time accuracy issue in your
>side?
>>>Have you tried other guest OS types other than Win7-32?
>>>
>>
>> I only verified it on win7-32 guest..
>> I will continue to verify it on other windows guest (I think windows
>> is enough, right?)
>
>No, I don't think Windows alone is sufficient for verification. People run all
>kinds of OSes as HVM guests, and your change should not negatively impact
>them. At the very least you want to also try Linux.
>

Cloud I use 'date' command to test it? As I only have server version of LINUX, 
no desktop version...


Quan

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [xen-4.4-testing test] 103756: trouble: blocked/broken/fail/pass

[osstest service owner]

flight 103756 xen-4.4-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/103756/

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-i386-xend   3 host-install(3)broken REGR. vs. 103438
 test-amd64-i386-xl-qemut-win7-amd64  3 host-install(3) broken REGR. vs. 103438
 test-amd64-i386-xl-qemuu-debianhvm-amd64 3 host-install(3) broken REGR. vs. 
103438
 test-amd64-amd64-xl-credit2   3 host-install(3)broken REGR. vs. 103438
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 3 host-install(3) broken REGR. vs. 
103438
 test-amd64-i386-qemut-rhel6hvm-amd  3 host-install(3)  broken REGR. vs. 103438
 test-amd64-amd64-i386-pvgrub  3 host-install(3)broken REGR. vs. 103438
 test-amd64-i386-qemuu-rhel6hvm-amd  3 host-install(3)  broken REGR. vs. 103438
 test-amd64-amd64-xl   3 host-install(3)broken REGR. vs. 103438
 test-amd64-i386-xl-qemut-debianhvm-amd64 3 host-install(3) broken REGR. vs. 
103438

Regressions which are regarded as allowable (not blocking):
 test-xtf-amd64-amd64-4   16 xtf/test-pv32pae-selftestfail  like 103438
 test-xtf-amd64-amd64-2   16 xtf/test-pv32pae-selftestfail  like 103438
 test-xtf-amd64-amd64-3   20 xtf/test-hvm32-invlpg~shadow fail  like 103438
 test-xtf-amd64-amd64-3 31 xtf/test-hvm32pae-invlpg~shadow fail like 103438
 test-xtf-amd64-amd64-2   52 leak-check/check fail  like 103438
 test-xtf-amd64-amd64-3   42 xtf/test-hvm64-invlpg~shadow fail  like 103438
 test-xtf-amd64-amd64-5   52 leak-check/check fail  like 103438
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 103438
 test-xtf-amd64-amd64-4   52 leak-check/check fail  like 103438
 test-xtf-amd64-amd64-3   52 leak-check/check fail  like 103438
 test-xtf-amd64-amd64-1   52 leak-check/check fail  like 103438
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stopfail like 103438
 test-armhf-armhf-xl-multivcpu 15 guest-start/debian.repeatfail like 103438

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-rumprun-amd64  1 build-check(1)   blocked  n/a
 test-amd64-i386-pv1 build-check(1)   blocked  n/a
 test-amd64-i386-rumprun-i386  1 build-check(1)   blocked  n/a
 test-amd64-i386-xend-qemut-winxpsp3  1 build-check(1)  blocked n/a
 test-xtf-amd64-amd64-4   10 xtf-fep  fail   never pass
 test-xtf-amd64-amd64-4   18 xtf/test-hvm32-cpuid-faulting fail  never pass
 test-xtf-amd64-amd64-1   10 xtf-fep  fail   never pass
 build-i386-rumprun7 xen-buildfail   never pass
 test-xtf-amd64-amd64-4 29 xtf/test-hvm32pae-cpuid-faulting fail never pass
 build-amd64-rumprun   7 xen-buildfail   never pass
 test-xtf-amd64-amd64-1   16 xtf/test-pv32pae-selftestfail   never pass
 test-xtf-amd64-amd64-1   18 xtf/test-hvm32-cpuid-faulting fail  never pass
 test-xtf-amd64-amd64-5   10 xtf-fep  fail   never pass
 test-xtf-amd64-amd64-2   10 xtf-fep  fail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-xtf-amd64-amd64-2   18 xtf/test-hvm32-cpuid-faulting fail  never pass
 test-xtf-amd64-amd64-4 35 xtf/test-hvm32pse-cpuid-faulting fail never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-xtf-amd64-amd64-1 29 xtf/test-hvm32pae-cpuid-faulting fail never pass
 test-xtf-amd64-amd64-4   39 xtf/test-hvm64-cpuid-faulting fail  never pass
 test-xtf-amd64-amd64-2 29 xtf/test-hvm32pae-cpuid-faulting fail never pass
 test-armhf-armhf-libvirt-qcow2  9 debian-di-installfail never pass
 test-xtf-amd64-amd64-2 35 xtf/test-hvm32pse-cpuid-faulting fail never pass
 test-xtf-amd64-amd64-2   39 xtf/test-hvm64-cpuid-faulting fail  never pass
 test-armhf-armhf-xl-vhd   9 debian-di-installfail   never pass
 test-xtf-amd64-amd64-1 35 xtf/test-hvm32pse-cpuid-faulting fail never pass
 test-xtf-amd64-amd64-3   10 xtf-fep  fail   never pass
 test-armhf-armhf-libvirt-raw  9 debian-di-installfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-intel 16 debian-hvm-install/l1/l2 fail never pass
 test-xtf-amd64-amd64-2   51 xtf/test-hvm64-xsa-195   fail   never pass
 test-xtf-amd64-amd64-1   39 xtf/test-hvm64-cpuid-faulting fail  never pass
 test-xtf-amd64-amd64-5   51 xtf/test-hvm64-xsa-195   fail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-amd64-amd64-

Re: [Xen-devel] Xen ARM - Exposing a PL011 to the guest

[Bhupinder Thakur]

Hi Stefano,

Thanks for a detailed explanation. I have some queries.

>
> Let me explain how the PV console protocol and drivers work, because
> they are a bit unusual. The first PV console is advertised via
> hvm_params. The guest calls:
>
>   hvm_get_parameter(HVM_PARAM_CONSOLE_EVTCHN, &v);
>   hvm_get_parameter(HVM_PARAM_CONSOLE_PFN, &v);
>
In drivers/tty/hvc/hvc_xen.c, I see that there are two console init
functions - xen_hvm_console_init () and xen_pv_console_init () and
either of them are called based on the xen_domain_type(). Can you
clarify which init () is called in which case?

> to get the two parameters to setup the ring and evtchn. If they are 0,
> the guest considers the first console unavailable. Other PV console
> rings, from the second onward, are advertised via xenstore like any
> other Xen PV protocols. In those cases, frontend and backend access
> xenstore to setup ring and event channel.
>
> The PV console backends are unusual too. xenconsoled, available on all
> Xen systems, is one process per host and can handle only one PV console
> per domain. Specifically, it is only able to deal with the first console.
> Domains that have multiple PV consoles require QEMU (not as an emulator,
> but as a PV backends provider). The toolstack writes "type" =
> "xenconsoled" or "ioemu" to distinguish PV consoles that xenconsoled or
> QEMU are supposed to handle. Ideally, we shouldn't require QEMU for
> pl011 PV consoles, but it wouldn't be the end of the world if we did.
>
> Additionally, Xen cannot speak xenstore. It can neither read nor write
> to it. I don't think we should add xenstore support to the hypervisor
> for this. We need to come up with a solution that doesn't require it.
>
> Finally, we cannot hijack one of the guest PV consoles, regardless of
> whether it's the first console or one of the others, because the guest
> can always try to use them at any time. We need a PV console reserved

I see that that there are xen events which can be allocated
(alloc_unbound_xen_event_channel ()). Should I use those kind of
events or just allocate a new event for each VM (similar to how the
console event channel is allocated) and intercept and process them in
the Xen HYP?

> for Xen-Dom0 communications on behalf of the guest. When a VM is created
> with "pl011=y", the toolstack needs to allocate one more page and evtchn
> for the exclusive hypervisor usage.  They are not going to be advertised
> to the guest as PV consoles; otherwise, the guest could rightfully
> access them.
>
There is a function alloc_magic_pages () (defined in
tools/libxc/xc_dom_arm.c), where the console PFN and event channel are
allocated for a new domain. So can I allocate a new PFN and the event
channel here for pl011?

> Both Xen and the PV console backend need access to the two numbers (pfn
> and evtchn) though. Xen doesn't do xenstore, so I suggest the toolstack
> should use another way to tell pfn and evtchn to Xen, maybe hvm_params.
> If we use hvm_params for this, we need two new hvm_params and Xen needs
> to unmap the pfn from the guest immediately, because we don't want the
> guest to have access to it.
>
This looks fine. We can add new hvm_params to communicate the PFN and
the event channel for Xen to use. These hvm_params can be called from
alloc_magic_pages().

> However, the PV console backend can access xenstore, so in that case, it
> is fine to write the pfn and evtchn of the PV console for pl011 to
> xenstore, paying attention at using the xenstore permissions
> appropriately. There are no reasons why the guest should have access to
> them; only the console backend should be able to read them. Given that
> the console backend has dom0 privileges, it is not a problem. I also
> suggest using new xenstore nodes, different from any of the existing PV
> console nodes.  For example:
>
> /local/domain/$DOMID/xen-console/$NUM/ring-ref
> /local/domain/$DOMID/xen-console/$NUM/port
>
> Where $DOMID is the guest domain id, and $NUM is the console number,
> starting from 0. If we use new hvm_parms for the pl011 PV console, we
> might get away without any xenstore stuff.
>
> For simplicity, given that xenconsoled doesn't support multiple PV
> consoles, we could setup the pl011 PV console *instead* of the regular
> PV console, hacking tools/console/daemon/io.c:domain_create_ring. It's
> safe if the toolstack doesn't provide a PV console. When pl011 is
> requested, libxl could set the pfn and evtchn hvm_params to 0 for the
> initial PV console. Eventually, it would be nice if xenconsoled was able
> to support both consoles at the same time.

Is domain_create_ring() responsible for setting up the ring and the
event channel for the first PV console? This function seems to refer
to xenstore for getting the shared page reference and the remote port.
Is this the same shared page which was set by libxl using hvm_param
for pfn (for guest to access).

Regards,
Bhupinder

___
Xen-devel mailing 

[Xen-devel] [PATCH 00/10] x86: register renaming (part I)

[Jan Beulich]

This is a first (of three, as far as current plans go) steps to do away
with misleading register names (eax instead of rax).

01: x86/MSR: introduce MSR access split/fold helpers
02: x86/guest-walk: use unambiguous register names
03: x86/shadow: use unambiguous register names
04: x86/oprofile: use unambiguous register names
05: x86/HVM: use unambiguous register names
06: x86/HVMemul: use unambiguous register names
07: x86/SVM: use unambiguous register names
(VMX counterpart omitted for now, as I'll need to re-base)
08: x86/vm-event: use unambiguous register names
09: x86/traps: use unambiguous register names
10: x86/misc: use unambiguous register names

Signed-off-by: Jan Beulich 


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] Xen: ARM: Zero reserved fields of xatp before making hypervisor call

[Juergen Gross]

On 20/12/16 06:02, Jiandi An wrote:
> On 12/19/16 12:49, Stefano Stabellini wrote:
>> On Mon, 19 Dec 2016, Juergen Gross wrote:
>>> On 19/12/16 03:56, Jiandi An wrote:
 Ensure all reserved fields of xatp are zero before making hypervisor
 call to XEN in xen_map_device_mmio().  xenmem_add_to_physmap_one() in
 XEN fails the mapping request if extra.res reserved field in xatp is
 not zero for XENMAPSPACE_dev_mmio request.

 Signed-off-by: Jiandi An 
 ---
  drivers/xen/arm-device.c | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/drivers/xen/arm-device.c b/drivers/xen/arm-device.c
 index 778acf8..208273b 100644
 --- a/drivers/xen/arm-device.c
 +++ b/drivers/xen/arm-device.c
 @@ -87,6 +87,9 @@ static int xen_map_device_mmio(const struct resource 
 *resources,
idxs[j] = XEN_PFN_DOWN(r->start) + j;
}
  
 +  /* Ensure reserved fields are set to zero */
 +  memset(&xatp, 0, sizeof(xatp));
 +
xatp.domid = DOMID_SELF;
xatp.size = nr;
xatp.space = XENMAPSPACE_dev_mmio;
>>>
>>> Instead of setting xatp to 0 in each iteration I'd prefer a static
>>> initialization of .domid and .space:
>>>
>>> struct xen_add_to_physmap_range xatp = {
>>> .domid = DOMID_SELF,
>>> .space = XENMAPSPACE_dev_mmio
>>> };
>>
>> +1
>>
> 
> Hi Juergen,
> 
> Thanks for you comments.  xatp is passed to XEN via the hypervisor call in 
> each loop.
> XEN touches xatp and returns it back.  For example XEN returns error of 
> underlying mapping call in the err[] array in xatp. (The err[] is not checked 
> after the hypervisor call returns and it's a bug to be addressed in a 
> separate patch)  XEN could theoretically corrupt xatp when it's returned.  
> And the loop would go on to the next iteration passing in whatever that's in 
> xatp returned by the previous hypervisor call.  Harder to debug in my opinion 
> if xatp get corrupted by XEN somehow when a bug is introduced in XEN.  At 
> first I put the memset of xatp at the beginning outside of the loop.  But I 
> thought it's better to initialize xatp that's passed in each time a 
> hypervisor call is made so we know exactly we set going into the hypervisor 
> call.
> 

It is very clearly specified which parameters are IN and which are OUT.
No trusting the hypervisor to follow this interface specification is
weird. Where do you want to stop?


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] x86/apicv: fix RTC periodic timer and apicv issue

[Jan Beulich]

>>> On 20.12.16 at 10:38,  wrote:
> On December 20, 2016 4:32 PM, Jan Beulich wrote:
> On 20.12.16 at 06:54,  wrote:
>>> On December 20, 2016 1:37 PM, Tian, Kevin wrote:
> From: Xuquan (Quan Xu) [mailto:xuqu...@huawei.com]
> Sent: Friday, December 16, 2016 5:40 PM
>
> From 89fffdd6b563b2723e24d17231715bb8c9f24f90 Mon Sep 17
>>00:00:00
2001
> From: Quan Xu 
> Date: Fri, 16 Dec 2016 17:24:01 +0800
> Subject: [PATCH v3] x86/apicv: fix RTC periodic timer and apicv
> issue
>
> When Xen apicv is enabled, wall clock time is faster on Windows7-32
> guest with high payload (with 2vCPU, captured from xentrace, in high
> payload, the count of IPI interrupt increases rapidly between these
> vCPUs).
>
> If IPI intrrupt (vector 0xe1) and periodic timer interrupt (vector
> 0xd1) are both pending (index of bit set in vIRR), unfortunately,
> the IPI intrrupt is high priority than periodic timer interrupt. Xen
> updates IPI interrupt bit set in vIRR to guest interrupt status
> (RVI) as a high priority and apicv (Virtual-Interrupt Delivery)
> delivers IPI interrupt within VMX non-root operation without a
> VM-Exit. Within VMX non-root operation, if periodic timer interrupt
> index of bit is set in vIRR and highest, the apicv delivers periodic
> timer interrupt within VMX non-root operation as well.
>
> But in current code, if Xen doesn't update periodic timer interrupt
> bit set in vIRR to guest interrupt status (RVI) directly, Xen is not
> aware of this case to decrease the count (pending_intr_nr) of
> pending periodic timer interrupt, then Xen will deliver a periodic
> timer interrupt
again.
>
> And that we update periodic timer interrupt in every VM-entry, there
> is a chance that already-injected instance (before EOI-induced exit
> happens) will incur another pending IRR setting if there is a
> VM-exit happens between virtual interrupt injection (vIRR->0,
> vISR->1) and EOI-induced exit (vISR->0), since pt_intr_post hasn't
> been invoked yet, then the guest receives more periodic timer
>>interrupt.
>
> So we set eoi_exit_bitmap for intack.vector when it's higher than
> pending periodic time interrupts. This way we can guarantee there's
> always a chance to post periodic time interrupts when periodic time
> interrupts becomes the highest one.
>
> Signed-off-by: Quan Xu 

I suppose you've verified this new version, but still would like get
your explicit confirmation - did you still see time accuracy issue in your
>>side?
Have you tried other guest OS types other than Win7-32?

>>>
>>> I only verified it on win7-32 guest..
>>> I will continue to verify it on other windows guest (I think windows
>>> is enough, right?)
>>
>>No, I don't think Windows alone is sufficient for verification. People run all
>>kinds of OSes as HVM guests, and your change should not negatively impact
>>them. At the very least you want to also try Linux.
> 
> Cloud I use 'date' command to test it? As I only have server version of 
> LINUX, no desktop version...

Well - I'm really not sure how to best test this.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] x86/apicv: fix RTC periodic timer and apicv issue

[Xuquan (Quan Xu)]

On December 20, 2016 4:54 PM, Tian, Kevin wrote:
>> From: Jan Beulich [mailto:jbeul...@suse.com]
>> Sent: Tuesday, December 20, 2016 4:35 PM
>>
>> >>> On 20.12.16 at 06:37,  wrote:
>> >>  From: Xuquan (Quan Xu) [mailto:xuqu...@huawei.com]
>> >> Sent: Friday, December 16, 2016 5:40 PM
>> >> -if (pt_vector != -1)
>> >> -vmx_set_eoi_exit_bitmap(v, pt_vector);
>> >> +if ( pt_vector != -1 ) {
>> >> +if ( intack.vector > pt_vector )
>> >> +vmx_set_eoi_exit_bitmap(v, intack.vector);
>> >> +else
>> >> +vmx_set_eoi_exit_bitmap(v, pt_vector);
>> >> +}
>> >
>> > Above can be simplified as one line change:
>> >if ( pt_vector != -1 )
>> >vmx_set_eoi_exit_bitmap(v, intack.vector);
>>
>> Hmm, I don't understand. Did you mean to use max() here? Or else how
>> is this an equivalent of the originally proposed code?
>>
>
>Original code is not 100% correct. The purpose is to set EOI exit bitmap for
>any vector which may block injection of pt_vector - give chance to recognize
>pt_vector in future intack and then do pt intr post. The simplified code
>achieves this effect same as original code if intack.vector >= vector. I cannot
>come up a case why intack.vector might be smaller than vector. If this case
>happens, we still need enable exit bitmap for intack.vector instead of
>pt_vector for said purpose while original code did it wrong.
>

Thanks for explaining this to me too!!
Your modification is better..

Quan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [xen-unstable-smoke test] 103771: tolerable all pass - PUSHED

[osstest service owner]

flight 103771 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/103771/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  d6ac8e22c7c5525db1da79fd1d1f03ee6b557f0d
baseline version:
 xen  157db407533fb55c0ce0c133991e3cb951b76484

Last test of basis   103761  2016-12-20 01:02:06 Z0 days
Testing same since   103771  2016-12-20 09:02:14 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Boris Ostrovsky 
  Haozhong Zhang 
  Jan Beulich 
  Kevin Tian 

jobs:
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  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 :

+ branch=xen-unstable-smoke
+ revision=d6ac8e22c7c5525db1da79fd1d1f03ee6b557f0d
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push xen-unstable-smoke 
d6ac8e22c7c5525db1da79fd1d1f03ee6b557f0d
+ branch=xen-unstable-smoke
+ revision=d6ac8e22c7c5525db1da79fd1d1f03ee6b557f0d
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . ./cri-common
++ . ./cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=xen
+ xenbranch=xen-unstable-smoke
+ qemuubranch=qemu-upstream-unstable
+ '[' xxen = xlinux ']'
+ linuxbranch=
+ '[' xqemu-upstream-unstable = x ']'
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable-smoke
+ prevxenbranch=xen-4.8-testing
+ '[' xd6ac8e22c7c5525db1da79fd1d1f03ee6b557f0d = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/qemu-xen-traditional.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/xtf.git
++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git
++ : git://xenbits.xen.org/xtf.git
++ : git://xenbits.xen.org/libvirt.git
++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git
++ : git://xenbits.xen.org/libvirt.git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/rumprun.git
++ : git://git.seabios.org/seabios.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/seabios.git
++ : git://xenbits.xen.org/osstest/seabios.git
++ : https://github.com/tianocore/edk2.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/linux-firmware.git
++ : osst...@xenbits.xen.org:/home/osstest/ext/linux-firmw

Re: [Xen-devel] [PATCH] Xen: ARM: Zero reserved fields of xatp before making hypervisor call

[Julien Grall]

Hi Jiandi,

Please respect the netiquette and wrap line to 70-75 characters.

On 20/12/2016 06:02, Jiandi An wrote:

On 12/19/16 12:49, Stefano Stabellini wrote:

On Mon, 19 Dec 2016, Juergen Gross wrote:

On 19/12/16 03:56, Jiandi An wrote:



Thanks for you comments.  xatp is passed to XEN via the hypervisor call in each 
loop.
XEN touches xatp and returns it back.  For example XEN returns error of 
underlying mapping call in the err[] array in xatp. (The err[] is not checked 
after the hypervisor call returns and it's a bug to be addressed in a separate 
patch)  XEN could theoretically corrupt xatp when it's returned.  And the loop 
would go on to the next iteration passing in whatever that's in xatp returned 
by the previous hypervisor call.  Harder to debug in my opinion if xatp get 
corrupted by XEN somehow when a bug is introduced in XEN.  At first I put the 
memset of xatp at the beginning outside of the loop.  But I thought it's better 
to initialize xatp that's passed in each time a hypervisor call is made so we 
know exactly we set going into the hypervisor call.


If you move struct xen_add_to_physmap_range in the loop, the compiler 
will initialize and zeroed for you the structure at each loop. I.e


for (i = 0; i < count; i++) {
struct xen_add_to_physmap_range xapt = 


}

Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 01/10] x86/MSR: introduce MSR access split/fold helpers

[Jan Beulich]

This is in preparation of eliminating the mis-naming of 64-bit fields
with 32-bit register names (eflags instead of rflags etc). Use the
guaranteed 32-bit underscore prefixed names for now where appropriate.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3695,12 +3695,9 @@ static uint64_t _hvm_rdtsc_intercept(voi
 
 void hvm_rdtsc_intercept(struct cpu_user_regs *regs)
 {
-uint64_t tsc = _hvm_rdtsc_intercept();
+msr_split(regs, _hvm_rdtsc_intercept());
 
-regs->eax = (uint32_t)tsc;
-regs->edx = (uint32_t)(tsc >> 32);
-
-HVMTRACE_2D(RDTSC, regs->eax, regs->edx);
+HVMTRACE_2D(RDTSC, regs->_eax, regs->_edx);
 }
 
 int hvm_msr_read_intercept(unsigned int msr, uint64_t *msr_content)
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1936,14 +1936,10 @@ static void svm_do_msr_access(struct cpu
 
 rc = hvm_msr_read_intercept(regs->_ecx, &msr_content);
 if ( rc == X86EMUL_OKAY )
-{
-regs->rax = (uint32_t)msr_content;
-regs->rdx = (uint32_t)(msr_content >> 32);
-}
+msr_split(regs, msr_content);
 }
 else
-rc = hvm_msr_write_intercept(regs->_ecx,
- (regs->rdx << 32) | regs->_eax, 1);
+rc = hvm_msr_write_intercept(regs->_ecx, msr_fold(regs), 1);
 
 if ( rc == X86EMUL_OKAY )
 __update_guest_eip(regs, inst_len);
@@ -2618,8 +2614,7 @@ void svm_vmexit_handler(struct cpu_user_
 if ( vmcb_get_cpl(vmcb) )
 hvm_inject_hw_exception(TRAP_gp_fault, 0);
 else if ( (inst_len = __get_instruction_length(v, INSTR_XSETBV)) &&
-  hvm_handle_xsetbv(regs->ecx,
-(regs->rdx << 32) | regs->_eax) == 0 )
+  hvm_handle_xsetbv(regs->_ecx, msr_fold(regs)) == 0 )
 __update_guest_eip(regs, inst_len);
 break;
 
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3626,22 +3626,18 @@ void vmx_vmexit_handler(struct cpu_user_
 case EXIT_REASON_MSR_READ:
 {
 uint64_t msr_content;
-if ( hvm_msr_read_intercept(regs->ecx, &msr_content) == X86EMUL_OKAY )
+if ( hvm_msr_read_intercept(regs->_ecx, &msr_content) == X86EMUL_OKAY )
 {
-regs->eax = (uint32_t)msr_content;
-regs->edx = (uint32_t)(msr_content >> 32);
+msr_split(regs, msr_content);
 update_guest_eip(); /* Safe: RDMSR */
 }
 break;
 }
+
 case EXIT_REASON_MSR_WRITE:
-{
-uint64_t msr_content;
-msr_content = ((uint64_t)regs->edx << 32) | (uint32_t)regs->eax;
-if ( hvm_msr_write_intercept(regs->ecx, msr_content, 1) == 
X86EMUL_OKAY )
+if ( hvm_msr_write_intercept(regs->_ecx, msr_fold(regs), 1) == 
X86EMUL_OKAY )
 update_guest_eip(); /* Safe: WRMSR */
 break;
-}
 
 case EXIT_REASON_VMXOFF:
 if ( nvmx_handle_vmxoff(regs) == X86EMUL_OKAY )
@@ -3802,8 +3798,7 @@ void vmx_vmexit_handler(struct cpu_user_
 break;
 
 case EXIT_REASON_XSETBV:
-if ( hvm_handle_xsetbv(regs->ecx,
-   (regs->rdx << 32) | regs->_eax) == 0 )
+if ( hvm_handle_xsetbv(regs->_ecx, msr_fold(regs)) == 0 )
 update_guest_eip(); /* Safe: XSETBV */
 break;
 
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -2322,15 +2322,11 @@ int nvmx_n2_vmexit_handler(struct cpu_us
 nvcpu->nv_vmexit_pending = 1;
 else
 {
-uint64_t tsc;
-
 /*
  * special handler is needed if L1 doesn't intercept rdtsc,
  * avoiding changing guest_tsc and messing up timekeeping in L1
  */
-tsc = hvm_get_guest_tsc(v) + get_vvmcs(v, TSC_OFFSET);
-regs->eax = (uint32_t)tsc;
-regs->edx = (uint32_t)(tsc >> 32);
+msr_split(regs, hvm_get_guest_tsc(v) + get_vvmcs(v, TSC_OFFSET));
 update_guest_eip();
 
 return 1;
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1918,13 +1918,10 @@ void pv_soft_rdtsc(struct vcpu *v, struc
 
 spin_unlock(&d->arch.vtsc_lock);
 
-now = gtime_to_gtsc(d, now);
-
-regs->eax = (uint32_t)now;
-regs->edx = (uint32_t)(now >> 32);
+msr_split(regs, gtime_to_gtsc(d, now));
 
 if ( rdtscp )
- regs->ecx =
+ regs->rcx =
  (d->arch.tsc_mode == TSC_MODE_PVRDTSCP) ? d->arch.incarnation : 0;
 }
 
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -3401,12 +3401,7 @@ if(rc) printk("%pv: %02x @ %08lx -> %d\n
 else if ( currd->arch.vtsc )
 pv_soft_rdtsc(curr, regs, 0);
 else
-{
-uint64_t val = rdtsc();
-
-regs->eax = (uint32_t)val;
-regs->edx = (uint32_t)(val >> 32);
-}
+msr_split(regs, rdtsc());
 

[Xen-devel] [PATCH 02/10] x86/guest-walk: use unambiguous register names

[Jan Beulich]

This is in preparation of eliminating the mis-naming of 64-bit fields
with 32-bit register names (eflags instead of rflags etc). Use the
guaranteed 32-bit underscore prefixed names for now where appropriate.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/mm/guest_walk.c
+++ b/xen/arch/x86/mm/guest_walk.c
@@ -196,7 +196,7 @@ guest_walk_tables(struct vcpu *v, struct
  *   - Page fault in kernel mode
  */
 smap = hvm_smap_enabled(v) &&
-   ((hvm_get_cpl(v) == 3) || !(regs->eflags & X86_EFLAGS_AC));
+   ((hvm_get_cpl(v) == 3) || !(regs->_eflags & X86_EFLAGS_AC));
 break;
 case SMAP_CHECK_ENABLED:
 smap = hvm_smap_enabled(v);



x86/guest-walk: use unambiguous register names

This is in preparation of eliminating the mis-naming of 64-bit fields
with 32-bit register names (eflags instead of rflags etc). Use the
guaranteed 32-bit underscore prefixed names for now where appropriate.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/mm/guest_walk.c
+++ b/xen/arch/x86/mm/guest_walk.c
@@ -196,7 +196,7 @@ guest_walk_tables(struct vcpu *v, struct
  *   - Page fault in kernel mode
  */
 smap = hvm_smap_enabled(v) &&
-   ((hvm_get_cpl(v) == 3) || !(regs->eflags & X86_EFLAGS_AC));
+   ((hvm_get_cpl(v) == 3) || !(regs->_eflags & X86_EFLAGS_AC));
 break;
 case SMAP_CHECK_ENABLED:
 smap = hvm_smap_enabled(v);
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 03/10] x86/shadow: use unambiguous register names

[Jan Beulich]

This is in preparation of eliminating the mis-naming of 64-bit fields
with 32-bit register names (eflags instead of rflags etc).

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -348,10 +348,10 @@ const struct x86_emulate_ops *shadow_ini
 }
 
 /* Attempt to prefetch whole instruction. */
-sh_ctxt->insn_buf_eip = regs->eip;
+sh_ctxt->insn_buf_eip = regs->rip;
 sh_ctxt->insn_buf_bytes =
 (!hvm_translate_linear_addr(
-x86_seg_cs, regs->eip, sizeof(sh_ctxt->insn_buf),
+x86_seg_cs, regs->rip, sizeof(sh_ctxt->insn_buf),
 hvm_access_insn_fetch, sh_ctxt, &addr) &&
  !hvm_fetch_from_guest_linear(
  sh_ctxt->insn_buf, addr, sizeof(sh_ctxt->insn_buf), 0, NULL))
@@ -374,18 +374,18 @@ void shadow_continue_emulation(struct sh
  * We don't refetch the segment bases, because we don't emulate
  * writes to segment registers
  */
-diff = regs->eip - sh_ctxt->insn_buf_eip;
+diff = regs->rip - sh_ctxt->insn_buf_eip;
 if ( diff > sh_ctxt->insn_buf_bytes )
 {
 /* Prefetch more bytes. */
 sh_ctxt->insn_buf_bytes =
 (!hvm_translate_linear_addr(
-x86_seg_cs, regs->eip, sizeof(sh_ctxt->insn_buf),
+x86_seg_cs, regs->rip, sizeof(sh_ctxt->insn_buf),
 hvm_access_insn_fetch, sh_ctxt, &addr) &&
  !hvm_fetch_from_guest_linear(
  sh_ctxt->insn_buf, addr, sizeof(sh_ctxt->insn_buf), 0, NULL))
 ? sizeof(sh_ctxt->insn_buf) : 0;
-sh_ctxt->insn_buf_eip = regs->eip;
+sh_ctxt->insn_buf_eip = regs->rip;
 }
 }
 
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -2872,7 +2872,7 @@ static int sh_page_fault(struct vcpu *v,
 #endif
 
 SHADOW_PRINTK("%pv va=%#lx err=%#x, rip=%lx\n",
-  v, va, regs->error_code, regs->eip);
+  v, va, regs->error_code, regs->rip);
 
 perfc_incr(shadow_fault);
 
@@ -3357,8 +3357,7 @@ static int sh_page_fault(struct vcpu *v,
 }
 }
 
-SHADOW_PRINTK("emulate: eip=%#lx esp=%#lx\n",
-  (unsigned long)regs->eip, (unsigned long)regs->esp);
+SHADOW_PRINTK("emulate: eip=%#lx esp=%#lx\n", regs->rip, regs->rsp);
 
 emul_ops = shadow_init_emulation(&emul_ctxt, regs);
 



x86/shadow: use unambiguous register names

This is in preparation of eliminating the mis-naming of 64-bit fields
with 32-bit register names (eflags instead of rflags etc).

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -348,10 +348,10 @@ const struct x86_emulate_ops *shadow_ini
 }
 
 /* Attempt to prefetch whole instruction. */
-sh_ctxt->insn_buf_eip = regs->eip;
+sh_ctxt->insn_buf_eip = regs->rip;
 sh_ctxt->insn_buf_bytes =
 (!hvm_translate_linear_addr(
-x86_seg_cs, regs->eip, sizeof(sh_ctxt->insn_buf),
+x86_seg_cs, regs->rip, sizeof(sh_ctxt->insn_buf),
 hvm_access_insn_fetch, sh_ctxt, &addr) &&
  !hvm_fetch_from_guest_linear(
  sh_ctxt->insn_buf, addr, sizeof(sh_ctxt->insn_buf), 0, NULL))
@@ -374,18 +374,18 @@ void shadow_continue_emulation(struct sh
  * We don't refetch the segment bases, because we don't emulate
  * writes to segment registers
  */
-diff = regs->eip - sh_ctxt->insn_buf_eip;
+diff = regs->rip - sh_ctxt->insn_buf_eip;
 if ( diff > sh_ctxt->insn_buf_bytes )
 {
 /* Prefetch more bytes. */
 sh_ctxt->insn_buf_bytes =
 (!hvm_translate_linear_addr(
-x86_seg_cs, regs->eip, sizeof(sh_ctxt->insn_buf),
+x86_seg_cs, regs->rip, sizeof(sh_ctxt->insn_buf),
 hvm_access_insn_fetch, sh_ctxt, &addr) &&
  !hvm_fetch_from_guest_linear(
  sh_ctxt->insn_buf, addr, sizeof(sh_ctxt->insn_buf), 0, NULL))
 ? sizeof(sh_ctxt->insn_buf) : 0;
-sh_ctxt->insn_buf_eip = regs->eip;
+sh_ctxt->insn_buf_eip = regs->rip;
 }
 }
 
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -2872,7 +2872,7 @@ static int sh_page_fault(struct vcpu *v,
 #endif
 
 SHADOW_PRINTK("%pv va=%#lx err=%#x, rip=%lx\n",
-  v, va, regs->error_code, regs->eip);
+  v, va, regs->error_code, regs->rip);
 
 perfc_incr(shadow_fault);
 
@@ -3357,8 +3357,7 @@ static int sh_page_fault(struct vcpu *v,
 }
 }
 
-SHADOW_PRINTK("emulate: eip=%#lx esp=%#lx\n",
-  (unsigned long)regs->eip, (unsigned long)regs->esp);
+SHADOW_PRINTK("emulate: eip=%#lx esp=%#lx\n", regs->rip, regs->rsp);
 
 emul_ops = shadow_init_emulation(&emul_ctxt, regs);
 
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 04/10] x86/oprofile: use unambiguous register names

[Jan Beulich]

This is in preparation of eliminating the mis-naming of 64-bit fields
with 32-bit register names (eflags instead of rflags etc).

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/oprofile/backtrace.c
+++ b/xen/arch/x86/oprofile/backtrace.c
@@ -150,7 +150,7 @@ static int valid_hypervisor_stack(const
 void xenoprof_backtrace(struct vcpu *vcpu, const struct cpu_user_regs *regs,
unsigned long depth, int mode)
 {
-const struct frame_head *head = (void *)regs->ebp;
+const struct frame_head *head = (void *)regs->rbp;
 
 if (mode > 1) {
 while (depth-- && valid_hypervisor_stack(head, regs))
--- a/xen/arch/x86/oprofile/op_model_athlon.c
+++ b/xen/arch/x86/oprofile/op_model_athlon.c
@@ -316,22 +316,20 @@ static int athlon_check_ctrs(unsigned in
uint64_t msr_content;
int i;
int ovf = 0;
-   unsigned long eip = regs->eip;
+   unsigned long eip = regs->rip;
int mode = 0;
struct vcpu *v = current;
struct cpu_user_regs *guest_regs = guest_cpu_user_regs();
unsigned int const nr_ctrs = model->num_counters;
 
if (!guest_mode(regs) &&
-   (regs->eip == (unsigned long)svm_stgi_label)) {
+   (eip == (unsigned long)svm_stgi_label)) {
/* SVM guest was running when NMI occurred */
ASSERT(is_hvm_vcpu(v));
-   eip = guest_regs->eip;
+   eip = guest_regs->rip;
mode = xenoprofile_get_mode(v, guest_regs);
-   } else {
-   eip = regs->eip;
+   } else
mode = xenoprofile_get_mode(v, regs);
-   }
 
for (i = 0 ; i < nr_ctrs; ++i) {
CTR_READ(msr_content, msrs, i);
--- a/xen/arch/x86/oprofile/op_model_p4.c
+++ b/xen/arch/x86/oprofile/op_model_p4.c
@@ -617,7 +617,7 @@ static int p4_check_ctrs(unsigned int co
uint64_t msr_content;
int i;
int ovf = 0;
-   unsigned long eip = regs->eip;
+   unsigned long eip = regs->rip;
int mode = xenoprofile_get_mode(current, regs);
 
stag = get_stagger();
--- a/xen/arch/x86/oprofile/op_model_ppro.c
+++ b/xen/arch/x86/oprofile/op_model_ppro.c
@@ -135,7 +135,7 @@ static int ppro_check_ctrs(unsigned int
u64 val;
int i;
int ovf = 0;
-   unsigned long eip = regs->eip;
+   unsigned long eip = regs->rip;
int mode = xenoprofile_get_mode(current, regs);
struct arch_msr_pair *msrs_content = vcpu_vpmu(current)->context;
 



x86/oprofile: use unambiguous register names

This is in preparation of eliminating the mis-naming of 64-bit fields
with 32-bit register names (eflags instead of rflags etc).

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/oprofile/backtrace.c
+++ b/xen/arch/x86/oprofile/backtrace.c
@@ -150,7 +150,7 @@ static int valid_hypervisor_stack(const
 void xenoprof_backtrace(struct vcpu *vcpu, const struct cpu_user_regs *regs,
unsigned long depth, int mode)
 {
-const struct frame_head *head = (void *)regs->ebp;
+const struct frame_head *head = (void *)regs->rbp;
 
 if (mode > 1) {
 while (depth-- && valid_hypervisor_stack(head, regs))
--- a/xen/arch/x86/oprofile/op_model_athlon.c
+++ b/xen/arch/x86/oprofile/op_model_athlon.c
@@ -316,22 +316,20 @@ static int athlon_check_ctrs(unsigned in
uint64_t msr_content;
int i;
int ovf = 0;
-   unsigned long eip = regs->eip;
+   unsigned long eip = regs->rip;
int mode = 0;
struct vcpu *v = current;
struct cpu_user_regs *guest_regs = guest_cpu_user_regs();
unsigned int const nr_ctrs = model->num_counters;
 
if (!guest_mode(regs) &&
-   (regs->eip == (unsigned long)svm_stgi_label)) {
+   (eip == (unsigned long)svm_stgi_label)) {
/* SVM guest was running when NMI occurred */
ASSERT(is_hvm_vcpu(v));
-   eip = guest_regs->eip;
+   eip = guest_regs->rip;
mode = xenoprofile_get_mode(v, guest_regs);
-   } else {
-   eip = regs->eip;
+   } else
mode = xenoprofile_get_mode(v, regs);
-   }
 
for (i = 0 ; i < nr_ctrs; ++i) {
CTR_READ(msr_content, msrs, i);
--- a/xen/arch/x86/oprofile/op_model_p4.c
+++ b/xen/arch/x86/oprofile/op_model_p4.c
@@ -617,7 +617,7 @@ static int p4_check_ctrs(unsigned int co
uint64_t msr_content;
int i;
int ovf = 0;
-   unsigned long eip = regs->eip;
+   unsigned long eip = regs->rip;
int mode = xenoprofile_get_mode(current, regs);
 
stag = get_stagger();
--- a/xen/arch/x86/oprofile/op_model_ppro.c
+++ b/xen/arch/x86/oprofile/op_model_ppro.c
@@ -135,7 +135,7 @@ static int ppro_check_ctrs(unsigned int
u64 val;
int i;
int ovf = 0;
-   unsigned long eip = regs->eip;
+   unsigned long eip = regs->rip;
int mode = xenoprofile_get_mode(current, regs);

[Xen-devel] [PATCH 05/10] x86/HVM: use unambiguous register names

[Jan Beulich]

This is in preparation of eliminating the mis-naming of 64-bit fields
with 32-bit register names (eflags instead of rflags etc). Use the
guaranteed 32-bit underscore prefixed names for now where appropriate.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -882,16 +882,16 @@ static int hvm_save_cpu_ctxt(struct doma
 ctxt.flags = XEN_X86_FPU_INITIALISED;
 }
 
-ctxt.rax = v->arch.user_regs.eax;
-ctxt.rbx = v->arch.user_regs.ebx;
-ctxt.rcx = v->arch.user_regs.ecx;
-ctxt.rdx = v->arch.user_regs.edx;
-ctxt.rbp = v->arch.user_regs.ebp;
-ctxt.rsi = v->arch.user_regs.esi;
-ctxt.rdi = v->arch.user_regs.edi;
-ctxt.rsp = v->arch.user_regs.esp;
-ctxt.rip = v->arch.user_regs.eip;
-ctxt.rflags = v->arch.user_regs.eflags;
+ctxt.rax = v->arch.user_regs.rax;
+ctxt.rbx = v->arch.user_regs.rbx;
+ctxt.rcx = v->arch.user_regs.rcx;
+ctxt.rdx = v->arch.user_regs.rdx;
+ctxt.rbp = v->arch.user_regs.rbp;
+ctxt.rsi = v->arch.user_regs.rsi;
+ctxt.rdi = v->arch.user_regs.rdi;
+ctxt.rsp = v->arch.user_regs.rsp;
+ctxt.rip = v->arch.user_regs.rip;
+ctxt.rflags = v->arch.user_regs.rflags;
 ctxt.r8  = v->arch.user_regs.r8;
 ctxt.r9  = v->arch.user_regs.r9;
 ctxt.r10 = v->arch.user_regs.r10;
@@ -1197,16 +1197,16 @@ static int hvm_load_cpu_ctxt(struct doma
 if ( xsave_area )
 xsave_area->xsave_hdr.xcomp_bv = 0;
 
-v->arch.user_regs.eax = ctxt.rax;
-v->arch.user_regs.ebx = ctxt.rbx;
-v->arch.user_regs.ecx = ctxt.rcx;
-v->arch.user_regs.edx = ctxt.rdx;
-v->arch.user_regs.ebp = ctxt.rbp;
-v->arch.user_regs.esi = ctxt.rsi;
-v->arch.user_regs.edi = ctxt.rdi;
-v->arch.user_regs.esp = ctxt.rsp;
-v->arch.user_regs.eip = ctxt.rip;
-v->arch.user_regs.eflags = ctxt.rflags | X86_EFLAGS_MBS;
+v->arch.user_regs.rax = ctxt.rax;
+v->arch.user_regs.rbx = ctxt.rbx;
+v->arch.user_regs.rcx = ctxt.rcx;
+v->arch.user_regs.rdx = ctxt.rdx;
+v->arch.user_regs.rbp = ctxt.rbp;
+v->arch.user_regs.rsi = ctxt.rsi;
+v->arch.user_regs.rdi = ctxt.rdi;
+v->arch.user_regs.rsp = ctxt.rsp;
+v->arch.user_regs.rip = ctxt.rip;
+v->arch.user_regs.rflags = ctxt.rflags | X86_EFLAGS_MBS;
 v->arch.user_regs.r8  = ctxt.r8;
 v->arch.user_regs.r9  = ctxt.r9;
 v->arch.user_regs.r10 = ctxt.r10;
@@ -1658,7 +1658,7 @@ void hvm_vcpu_down(struct vcpu *v)
 }
 }
 
-void hvm_hlt(unsigned long rflags)
+void hvm_hlt(unsigned int eflags)
 {
 struct vcpu *curr = current;
 
@@ -1670,7 +1670,7 @@ void hvm_hlt(unsigned long rflags)
  * want to shut down. In a real processor, NMIs are the only way to break
  * out of this.
  */
-if ( unlikely(!(rflags & X86_EFLAGS_IF)) )
+if ( unlikely(!(eflags & X86_EFLAGS_IF)) )
 return hvm_vcpu_down(curr);
 
 do_sched_op(SCHEDOP_block, guest_handle_from_ptr(NULL, void));
@@ -2901,7 +2901,7 @@ void hvm_task_switch(
 struct segment_register gdt, tr, prev_tr, segr;
 struct desc_struct *optss_desc = NULL, *nptss_desc = NULL, tss_desc;
 bool_t otd_writable, ntd_writable;
-unsigned long eflags;
+unsigned int eflags;
 pagefault_info_t pfinfo;
 int exn_raised, rc;
 struct {
@@ -2975,20 +2975,20 @@ void hvm_task_switch(
 if ( rc != HVMCOPY_okay )
 goto out;
 
-eflags = regs->eflags;
+eflags = regs->_eflags;
 if ( taskswitch_reason == TSW_iret )
 eflags &= ~X86_EFLAGS_NT;
 
-tss.eip= regs->eip;
+tss.eip= regs->_eip;
 tss.eflags = eflags;
-tss.eax= regs->eax;
-tss.ecx= regs->ecx;
-tss.edx= regs->edx;
-tss.ebx= regs->ebx;
-tss.esp= regs->esp;
-tss.ebp= regs->ebp;
-tss.esi= regs->esi;
-tss.edi= regs->edi;
+tss.eax= regs->_eax;
+tss.ecx= regs->_ecx;
+tss.edx= regs->_edx;
+tss.ebx= regs->_ebx;
+tss.esp= regs->_esp;
+tss.ebp= regs->_ebp;
+tss.esi= regs->_esi;
+tss.edi= regs->_edi;
 
 hvm_get_segment_register(v, x86_seg_es, &segr);
 tss.es = segr.sel;
@@ -3032,16 +3032,16 @@ void hvm_task_switch(
 if ( hvm_set_cr3(tss.cr3, 1) )
 goto out;
 
-regs->eip= tss.eip;
-regs->eflags = tss.eflags | 2;
-regs->eax= tss.eax;
-regs->ecx= tss.ecx;
-regs->edx= tss.edx;
-regs->ebx= tss.ebx;
-regs->esp= tss.esp;
-regs->ebp= tss.ebp;
-regs->esi= tss.esi;
-regs->edi= tss.edi;
+regs->rip= tss.eip;
+regs->rflags = tss.eflags | 2;
+regs->rax= tss.eax;
+regs->rcx= tss.ecx;
+regs->rdx= tss.edx;
+regs->rbx= tss.ebx;
+regs->rsp= tss.esp;
+regs->rbp= tss.ebp;
+regs->rsi= tss.esi;
+regs->rdi= tss.edi;
 
 exn_raised = 0;
 if ( hvm_load_segment_sel

[Xen-devel] [PATCH 06/10] x86/HVMemul: use unambiguous register names

[Jan Beulich]

This is in preparation of eliminating the mis-naming of 64-bit fields
with 32-bit register names (eflags instead of rflags etc). Use the
guaranteed 32-bit underscore prefixed names for now where appropriate.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -442,7 +442,7 @@ static int hvmemul_linear_to_phys(
 }
 
 /* Reverse mode if this is a backwards multi-iteration string operation. */
-reverse = (hvmemul_ctxt->ctxt.regs->eflags & X86_EFLAGS_DF) && (*reps > 1);
+reverse = (hvmemul_ctxt->ctxt.regs->_eflags & X86_EFLAGS_DF) && (*reps > 
1);
 
 if ( reverse && ((PAGE_SIZE - offset) < bytes_per_rep) )
 {
@@ -539,7 +539,7 @@ static int hvmemul_virtual_to_linear(
 if ( IS_ERR(reg) )
 return -PTR_ERR(reg);
 
-if ( (hvmemul_ctxt->ctxt.regs->eflags & X86_EFLAGS_DF) && (*reps > 1) )
+if ( (hvmemul_ctxt->ctxt.regs->_eflags & X86_EFLAGS_DF) && (*reps > 1) )
 {
 /*
  * x86_emulate() clips the repetition count to ensure we don't wrap
@@ -1074,7 +1074,7 @@ static int hvmemul_rep_ins(
 return X86EMUL_UNHANDLEABLE;
 
 return hvmemul_do_pio_addr(src_port, reps, bytes_per_rep, IOREQ_READ,
-   !!(ctxt->regs->eflags & X86_EFLAGS_DF), gpa);
+   !!(ctxt->regs->_eflags & X86_EFLAGS_DF), gpa);
 }
 
 static int hvmemul_rep_outs_set_context(
@@ -1143,7 +1143,7 @@ static int hvmemul_rep_outs(
 return X86EMUL_UNHANDLEABLE;
 
 return hvmemul_do_pio_addr(dst_port, reps, bytes_per_rep, IOREQ_WRITE,
-   !!(ctxt->regs->eflags & X86_EFLAGS_DF), gpa);
+   !!(ctxt->regs->_eflags & X86_EFLAGS_DF), gpa);
 }
 
 static int hvmemul_rep_movs(
@@ -1162,7 +1162,7 @@ static int hvmemul_rep_movs(
 paddr_t sgpa, dgpa;
 uint32_t pfec = PFEC_page_present;
 p2m_type_t sp2mt, dp2mt;
-int rc, df = !!(ctxt->regs->eflags & X86_EFLAGS_DF);
+int rc, df = !!(ctxt->regs->_eflags & X86_EFLAGS_DF);
 char *buf;
 
 rc = hvmemul_virtual_to_linear(
@@ -1316,7 +1316,7 @@ static int hvmemul_rep_stos(
 unsigned long addr, bytes;
 paddr_t gpa;
 p2m_type_t p2mt;
-bool_t df = !!(ctxt->regs->eflags & X86_EFLAGS_DF);
+bool_t df = !!(ctxt->regs->_eflags & X86_EFLAGS_DF);
 int rc = hvmemul_virtual_to_linear(seg, offset, bytes_per_rep, reps,
hvm_access_write, hvmemul_ctxt, &addr);
 
@@ -1766,7 +1766,7 @@ static int _hvm_emulate_one(struct hvm_e
 if ( hvmemul_ctxt->ctxt.retire.hlt &&
  !hvm_local_events_need_delivery(curr) )
 {
-hvm_hlt(regs->eflags);
+hvm_hlt(regs->_eflags);
 }
 
 return X86EMUL_OKAY;
@@ -1930,7 +1930,7 @@ void hvm_emulate_init_per_insn(
 if ( hvmemul_ctxt->seg_reg[x86_seg_ss].attr.fields.dpl == 3 )
 pfec |= PFEC_user_mode;
 
-hvmemul_ctxt->insn_buf_eip = hvmemul_ctxt->ctxt.regs->eip;
+hvmemul_ctxt->insn_buf_eip = hvmemul_ctxt->ctxt.regs->rip;
 if ( !insn_bytes )
 {
 hvmemul_ctxt->insn_buf_bytes =
--- a/xen/arch/x86/hvm/io.c
+++ b/xen/arch/x86/hvm/io.c
@@ -136,7 +136,7 @@ int handle_pio(uint16_t port, unsigned i
 ASSERT((size - 1) < 4 && size != 3);
 
 if ( dir == IOREQ_WRITE )
-data = guest_cpu_user_regs()->eax;
+data = guest_cpu_user_regs()->_eax;
 
 rc = hvmemul_do_pio_buffer(port, size, dir, &data);
 



x86/HVMemul: use unambiguous register names

This is in preparation of eliminating the mis-naming of 64-bit fields
with 32-bit register names (eflags instead of rflags etc). Use the
guaranteed 32-bit underscore prefixed names for now where appropriate.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -442,7 +442,7 @@ static int hvmemul_linear_to_phys(
 }
 
 /* Reverse mode if this is a backwards multi-iteration string operation. */
-reverse = (hvmemul_ctxt->ctxt.regs->eflags & X86_EFLAGS_DF) && (*reps > 1);
+reverse = (hvmemul_ctxt->ctxt.regs->_eflags & X86_EFLAGS_DF) && (*reps > 
1);
 
 if ( reverse && ((PAGE_SIZE - offset) < bytes_per_rep) )
 {
@@ -539,7 +539,7 @@ static int hvmemul_virtual_to_linear(
 if ( IS_ERR(reg) )
 return -PTR_ERR(reg);
 
-if ( (hvmemul_ctxt->ctxt.regs->eflags & X86_EFLAGS_DF) && (*reps > 1) )
+if ( (hvmemul_ctxt->ctxt.regs->_eflags & X86_EFLAGS_DF) && (*reps > 1) )
 {
 /*
  * x86_emulate() clips the repetition count to ensure we don't wrap
@@ -1074,7 +1074,7 @@ static int hvmemul_rep_ins(
 return X86EMUL_UNHANDLEABLE;
 
 return hvmemul_do_pio_addr(src_port, reps, bytes_per_rep, IOREQ_READ,
-   !!(ctxt->regs->eflags & X86_EFLAGS_DF), gpa);
+   !!(ctxt->regs->_eflags & X86_EFLAGS_DF), gpa);
 }
 
 static int hvmemul_rep_outs_set_context(
@@ -1143,7 +1143,7 @@ static int hvmemul_rep_outs(
 return X86EMUL_UNHANDL

[Xen-devel] [PATCH 07/10] x86/SVM: use unambiguous register names

[Jan Beulich]

This is in preparation of eliminating the mis-naming of 64-bit fields
with 32-bit register names (eflags instead of rflags etc). Use the
guaranteed 32-bit underscore prefixed names for now where appropriate.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -322,10 +322,10 @@ static int nsvm_vcpu_hostrestore(struct
 if (rc != X86EMUL_OKAY)
 gdprintk(XENLOG_ERR, "hvm_set_cr3 failed, rc: %u\n", rc);
 
-regs->eax = n1vmcb->rax;
-regs->esp = n1vmcb->rsp;
-regs->eip = n1vmcb->rip;
-regs->eflags = n1vmcb->rflags;
+regs->rax = n1vmcb->rax;
+regs->rsp = n1vmcb->rsp;
+regs->rip = n1vmcb->rip;
+regs->rflags = n1vmcb->rflags;
 n1vmcb->_dr7 = 0; /* disable all breakpoints */
 n1vmcb->_cpl = 0;
 
@@ -653,10 +653,10 @@ static int nsvm_vmcb_prepare4vmrun(struc
 }
 
 /* Switch guest registers to l2 guest */
-regs->eax = ns_vmcb->rax;
-regs->eip = ns_vmcb->rip;
-regs->esp = ns_vmcb->rsp;
-regs->eflags = ns_vmcb->rflags;
+regs->rax = ns_vmcb->rax;
+regs->rip = ns_vmcb->rip;
+regs->rsp = ns_vmcb->rsp;
+regs->rflags = ns_vmcb->rflags;
 
 #undef vcleanbit_set
 return 0;
@@ -975,7 +975,7 @@ nsvm_vmcb_guest_intercepts_exitcode(stru
 break;
 ns_vmcb = nv->nv_vvmcx;
 vmexits = nsvm_vmcb_guest_intercepts_msr(svm->ns_cached_msrpm,
-regs->ecx, ns_vmcb->exitinfo1 != 0);
+regs->_ecx, ns_vmcb->exitinfo1 != 0);
 if (vmexits == NESTEDHVM_VMEXIT_HOST)
 return 0;
 break;
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -110,12 +110,12 @@ void __update_guest_eip(struct cpu_user_
 
 ASSERT(regs == guest_cpu_user_regs());
 
-regs->eip += inst_len;
-regs->eflags &= ~X86_EFLAGS_RF;
+regs->rip += inst_len;
+regs->_eflags &= ~X86_EFLAGS_RF;
 
 curr->arch.hvm_svm.vmcb->interrupt_shadow = 0;
 
-if ( regs->eflags & X86_EFLAGS_TF )
+if ( regs->_eflags & X86_EFLAGS_TF )
 hvm_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
 }
 
@@ -520,7 +520,7 @@ static int svm_guest_x86_mode(struct vcp
 
 if ( unlikely(!(v->arch.hvm_vcpu.guest_cr[0] & X86_CR0_PE)) )
 return 0;
-if ( unlikely(guest_cpu_user_regs()->eflags & X86_EFLAGS_VM) )
+if ( unlikely(guest_cpu_user_regs()->_eflags & X86_EFLAGS_VM) )
 return 1;
 if ( hvm_long_mode_enabled(v) && likely(vmcb->cs.attr.fields.l) )
 return 8;
@@ -1226,7 +1226,7 @@ static void svm_inject_event(const struc
 switch ( _event.vector )
 {
 case TRAP_debug:
-if ( regs->eflags & X86_EFLAGS_TF )
+if ( regs->_eflags & X86_EFLAGS_TF )
 {
 __restore_debug_registers(vmcb, curr);
 vmcb_set_dr6(vmcb, vmcb_get_dr6(vmcb) | 0x4000);
@@ -1635,18 +1635,18 @@ static void svm_vmexit_do_cpuid(struct c
 if ( (inst_len = __get_instruction_length(current, INSTR_CPUID)) == 0 )
 return;
 
-eax = regs->eax;
-ebx = regs->ebx;
-ecx = regs->ecx;
-edx = regs->edx;
+eax = regs->_eax;
+ebx = regs->_ebx;
+ecx = regs->_ecx;
+edx = regs->_edx;
 
 hvm_cpuid(regs->_eax, &eax, &ebx, &ecx, &edx);
 HVMTRACE_5D(CPUID, regs->_eax, eax, ebx, ecx, edx);
 
-regs->eax = eax;
-regs->ebx = ebx;
-regs->ecx = ecx;
-regs->edx = edx;
+regs->rax = eax;
+regs->rbx = ebx;
+regs->rcx = ecx;
+regs->rdx = edx;
 
 __update_guest_eip(regs, inst_len);
 }
@@ -2011,7 +2011,7 @@ static void svm_vmexit_do_hlt(struct vmc
 return;
 __update_guest_eip(regs, inst_len);
 
-hvm_hlt(regs->eflags);
+hvm_hlt(regs->_eflags);
 }
 
 static void svm_vmexit_do_rdtsc(struct cpu_user_regs *regs)
@@ -2332,13 +2332,11 @@ void svm_vmexit_handler(struct cpu_user_
 if ( hvm_long_mode_enabled(v) )
 HVMTRACE_ND(VMEXIT64, vcpu_guestmode ? TRC_HVM_NESTEDFLAG : 0,
 1/*cycles*/, 3, exit_reason,
-(uint32_t)regs->eip, (uint32_t)((uint64_t)regs->eip >> 32),
-0, 0, 0);
+regs->_eip, regs->rip >> 32, 0, 0, 0);
 else
 HVMTRACE_ND(VMEXIT, vcpu_guestmode ? TRC_HVM_NESTEDFLAG : 0,
 1/*cycles*/, 2, exit_reason,
-(uint32_t)regs->eip,
-0, 0, 0, 0);
+regs->_eip, 0, 0, 0, 0);
 
 if ( vcpu_guestmode ) {
 enum nestedhvm_vmexits nsret;
@@ -2476,9 +2474,8 @@ void svm_vmexit_handler(struct cpu_user_
 regs->error_code = vmcb->exitinfo1;
 HVM_DBG_LOG(DBG_LEVEL_VMMU,
 "eax=%lx, ebx=%lx, ecx=%lx, edx=%lx, esi=%lx, edi=%lx",
-(unsigned long)regs->eax, (unsigned long)regs->ebx,
-(unsigned long)regs->ecx, (unsigned long)regs->edx,
-(unsigned long)regs->esi, (unsigned long)regs->edi);
+regs->rax, regs->rbx, regs->rcx,
+  

[Xen-devel] [PATCH 08/10] x86/vm-event: use unambiguous register names

[Jan Beulich]

This is in preparation of eliminating the mis-naming of 64-bit fields
with 32-bit register names (eflags instead of rflags etc).

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/vm_event.c
+++ b/xen/arch/x86/vm_event.c
@@ -112,14 +112,14 @@ void vm_event_set_registers(struct vcpu
 {
 ASSERT(atomic_read(&v->vm_event_pause_count));
 
-v->arch.user_regs.eax = rsp->data.regs.x86.rax;
-v->arch.user_regs.ebx = rsp->data.regs.x86.rbx;
-v->arch.user_regs.ecx = rsp->data.regs.x86.rcx;
-v->arch.user_regs.edx = rsp->data.regs.x86.rdx;
-v->arch.user_regs.esp = rsp->data.regs.x86.rsp;
-v->arch.user_regs.ebp = rsp->data.regs.x86.rbp;
-v->arch.user_regs.esi = rsp->data.regs.x86.rsi;
-v->arch.user_regs.edi = rsp->data.regs.x86.rdi;
+v->arch.user_regs.rax = rsp->data.regs.x86.rax;
+v->arch.user_regs.rbx = rsp->data.regs.x86.rbx;
+v->arch.user_regs.rcx = rsp->data.regs.x86.rcx;
+v->arch.user_regs.rdx = rsp->data.regs.x86.rdx;
+v->arch.user_regs.rsp = rsp->data.regs.x86.rsp;
+v->arch.user_regs.rbp = rsp->data.regs.x86.rbp;
+v->arch.user_regs.rsi = rsp->data.regs.x86.rsi;
+v->arch.user_regs.rdi = rsp->data.regs.x86.rdi;
 
 v->arch.user_regs.r8 = rsp->data.regs.x86.r8;
 v->arch.user_regs.r9 = rsp->data.regs.x86.r9;
@@ -130,8 +130,8 @@ void vm_event_set_registers(struct vcpu
 v->arch.user_regs.r14 = rsp->data.regs.x86.r14;
 v->arch.user_regs.r15 = rsp->data.regs.x86.r15;
 
-v->arch.user_regs.eflags = rsp->data.regs.x86.rflags;
-v->arch.user_regs.eip = rsp->data.regs.x86.rip;
+v->arch.user_regs.rflags = rsp->data.regs.x86.rflags;
+v->arch.user_regs.rip = rsp->data.regs.x86.rip;
 }
 
 void vm_event_monitor_next_interrupt(struct vcpu *v)
@@ -151,14 +151,14 @@ void vm_event_fill_regs(vm_event_request
 /* Architecture-specific vmcs/vmcb bits */
 hvm_funcs.save_cpu_ctxt(curr, &ctxt);
 
-req->data.regs.x86.rax = regs->eax;
-req->data.regs.x86.rcx = regs->ecx;
-req->data.regs.x86.rdx = regs->edx;
-req->data.regs.x86.rbx = regs->ebx;
-req->data.regs.x86.rsp = regs->esp;
-req->data.regs.x86.rbp = regs->ebp;
-req->data.regs.x86.rsi = regs->esi;
-req->data.regs.x86.rdi = regs->edi;
+req->data.regs.x86.rax = regs->rax;
+req->data.regs.x86.rcx = regs->rcx;
+req->data.regs.x86.rdx = regs->rdx;
+req->data.regs.x86.rbx = regs->rbx;
+req->data.regs.x86.rsp = regs->rsp;
+req->data.regs.x86.rbp = regs->rbp;
+req->data.regs.x86.rsi = regs->rsi;
+req->data.regs.x86.rdi = regs->rdi;
 
 req->data.regs.x86.r8  = regs->r8;
 req->data.regs.x86.r9  = regs->r9;
@@ -169,8 +169,8 @@ void vm_event_fill_regs(vm_event_request
 req->data.regs.x86.r14 = regs->r14;
 req->data.regs.x86.r15 = regs->r15;
 
-req->data.regs.x86.rflags = regs->eflags;
-req->data.regs.x86.rip= regs->eip;
+req->data.regs.x86.rflags = regs->rflags;
+req->data.regs.x86.rip= regs->rip;
 
 req->data.regs.x86.dr7 = curr->arch.debugreg[7];
 req->data.regs.x86.cr0 = ctxt.cr0;



x86/vm-event: use unambiguous register names

This is in preparation of eliminating the mis-naming of 64-bit fields
with 32-bit register names (eflags instead of rflags etc).

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/vm_event.c
+++ b/xen/arch/x86/vm_event.c
@@ -112,14 +112,14 @@ void vm_event_set_registers(struct vcpu
 {
 ASSERT(atomic_read(&v->vm_event_pause_count));
 
-v->arch.user_regs.eax = rsp->data.regs.x86.rax;
-v->arch.user_regs.ebx = rsp->data.regs.x86.rbx;
-v->arch.user_regs.ecx = rsp->data.regs.x86.rcx;
-v->arch.user_regs.edx = rsp->data.regs.x86.rdx;
-v->arch.user_regs.esp = rsp->data.regs.x86.rsp;
-v->arch.user_regs.ebp = rsp->data.regs.x86.rbp;
-v->arch.user_regs.esi = rsp->data.regs.x86.rsi;
-v->arch.user_regs.edi = rsp->data.regs.x86.rdi;
+v->arch.user_regs.rax = rsp->data.regs.x86.rax;
+v->arch.user_regs.rbx = rsp->data.regs.x86.rbx;
+v->arch.user_regs.rcx = rsp->data.regs.x86.rcx;
+v->arch.user_regs.rdx = rsp->data.regs.x86.rdx;
+v->arch.user_regs.rsp = rsp->data.regs.x86.rsp;
+v->arch.user_regs.rbp = rsp->data.regs.x86.rbp;
+v->arch.user_regs.rsi = rsp->data.regs.x86.rsi;
+v->arch.user_regs.rdi = rsp->data.regs.x86.rdi;
 
 v->arch.user_regs.r8 = rsp->data.regs.x86.r8;
 v->arch.user_regs.r9 = rsp->data.regs.x86.r9;
@@ -130,8 +130,8 @@ void vm_event_set_registers(struct vcpu
 v->arch.user_regs.r14 = rsp->data.regs.x86.r14;
 v->arch.user_regs.r15 = rsp->data.regs.x86.r15;
 
-v->arch.user_regs.eflags = rsp->data.regs.x86.rflags;
-v->arch.user_regs.eip = rsp->data.regs.x86.rip;
+v->arch.user_regs.rflags = rsp->data.regs.x86.rflags;
+v->arch.user_regs.rip = rsp->data.regs.x86.rip;
 }
 
 void vm_event_monitor_next_interrupt(struct vcpu *v)
@@ -151,14 +151,14 @@ void vm_event_fill_regs(vm_event_request
 /* Architecture-specific vmcs/vmcb bits */
 hvm_funcs.save_cpu_ctxt(curr, &ctxt

[Xen-devel] [PATCH 09/10] x86/traps: use unambiguous register names

[Jan Beulich]

This is in preparation of eliminating the mis-naming of 64-bit fields
with 32-bit register names (eflags instead of rflags etc). Use the
guaranteed 32-bit underscore prefixed names for now where appropriate.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -202,7 +202,7 @@ static void show_guest_stack(struct vcpu
 return;
 }
 
-stack = (unsigned long *)regs->esp;
+stack = (unsigned long *)regs->rsp;
 printk("Guest stack trace from "__OP"sp=%p:\n  ", stack);
 
 if ( !access_ok(stack, sizeof(*stack)) )
@@ -367,8 +367,8 @@ static void _show_trace(unsigned long sp
 break;
 frame = (unsigned long *)next;
 next  = frame[0];
-addr  = frame[(offsetof(struct cpu_user_regs, eip) -
-   offsetof(struct cpu_user_regs, ebp))
+addr  = frame[(offsetof(struct cpu_user_regs, rip) -
+   offsetof(struct cpu_user_regs, rbp))
  / BYTES_PER_LONG];
 }
 else
@@ -623,7 +623,7 @@ void fatal_trap(const struct cpu_user_re
 panic("FATAL TRAP: vector = %d (%s)\n"
   "[error_code=%04x] %s",
   trapnr, trapstr(trapnr), regs->error_code,
-  (regs->eflags & X86_EFLAGS_IF) ? "" : ", IN INTERRUPT CONTEXT");
+  (regs->_eflags & X86_EFLAGS_IF) ? "" : ", IN INTERRUPT CONTEXT");
 }
 
 void pv_inject_event(const struct x86_event *event)
@@ -663,7 +663,7 @@ void pv_inject_event(const struct x86_ev
 trace_pv_page_fault(event->cr2, error_code);
 }
 else
-trace_pv_trap(vector, regs->eip, use_error_code, error_code);
+trace_pv_trap(vector, regs->rip, use_error_code, error_code);
 
 if ( use_error_code )
 {
@@ -697,11 +697,11 @@ static inline void do_guest_trap(unsigne
 pv_inject_event(&event);
 }
 
-static void instruction_done(struct cpu_user_regs *regs, unsigned long eip)
+static void instruction_done(struct cpu_user_regs *regs, unsigned long rip)
 {
-regs->eip = eip;
-regs->eflags &= ~X86_EFLAGS_RF;
-if ( regs->eflags & X86_EFLAGS_TF )
+regs->rip = rip;
+regs->_eflags &= ~X86_EFLAGS_RF;
+if ( regs->_eflags & X86_EFLAGS_TF )
 {
 current->arch.debugreg[6] |= DR_STEP | DR_STATUS_RESERVED_ONE;
 do_guest_trap(TRAP_debug, regs);
@@ -799,12 +799,12 @@ void do_trap(struct cpu_user_regs *regs)
 return;
 }
 
-if ( likely((fixup = search_exception_table(regs->eip)) != 0) )
+if ( likely((fixup = search_exception_table(regs->rip)) != 0) )
 {
 dprintk(XENLOG_ERR, "Trap %d: %p -> %p\n",
-trapnr, _p(regs->eip), _p(fixup));
-this_cpu(last_extable_addr) = regs->eip;
-regs->eip = fixup;
+trapnr, _p(regs->rip), _p(fixup));
+this_cpu(last_extable_addr) = regs->rip;
+regs->rip = fixup;
 return;
 }
 
@@ -1042,10 +1042,10 @@ void pv_cpuid(struct cpu_user_regs *regs
 struct vcpu *curr = current;
 struct domain *currd = curr->domain;
 
-leaf = a = regs->eax;
-b = regs->ebx;
-subleaf = c = regs->ecx;
-d = regs->edx;
+leaf = a = regs->_eax;
+b = regs->_ebx;
+subleaf = c = regs->_ecx;
+d = regs->_edx;
 
 if ( cpuid_hypervisor_leaves(leaf, subleaf, &a, &b, &c, &d) )
 goto out;
@@ -1065,10 +1065,10 @@ void pv_cpuid(struct cpu_user_regs *regs
 limit = cpuid_eax(limit);
 if ( leaf > limit )
 {
-regs->eax = 0;
-regs->ebx = 0;
-regs->ecx = 0;
-regs->edx = 0;
+regs->rax = 0;
+regs->rbx = 0;
+regs->rcx = 0;
+regs->rdx = 0;
 return;
 }
 }
@@ -1382,10 +1382,10 @@ void pv_cpuid(struct cpu_user_regs *regs
 }
 
  out:
-regs->eax = a;
-regs->ebx = b;
-regs->ecx = c;
-regs->edx = d;
+regs->rax = a;
+regs->rbx = b;
+regs->rcx = c;
+regs->rdx = d;
 }
 
 static int emulate_invalid_rdtscp(struct cpu_user_regs *regs)
@@ -1394,7 +1394,7 @@ static int emulate_invalid_rdtscp(struct
 unsigned long eip, rc;
 struct vcpu *v = current;
 
-eip = regs->eip;
+eip = regs->rip;
 if ( (rc = copy_from_user(opcode, (char *)eip, sizeof(opcode))) != 0 )
 {
 pv_inject_page_fault(0, eip + sizeof(opcode) - rc);
@@ -1413,7 +1413,7 @@ static int emulate_forced_invalid_op(str
 char sig[5], instr[2];
 unsigned long eip, rc;
 
-eip = regs->eip;
+eip = regs->rip;
 
 /* Check for forced emulation signature: ud2 ; .ascii "xen". */
 if ( (rc = copy_from_user(sig, (char *)eip, sizeof(sig))) != 0 )
@@ -1437,7 +1437,7 @@ static int emulate_forced_invalid_op(str
 /* If cpuid faulting is enabled and CPL>0 inject a #GP in place of #UD. */
 if ( current->arch.cpuid_faulting && !guest_kernel_mode(current, regs) )
 {
-regs->eip = eip;
+regs->rip = eip;
 do_guest_trap(TRAP

[Xen-devel] [PATCH 10/10] x86/misc: use unambiguous register names

[Jan Beulich]

This is in preparation of eliminating the mis-naming of 64-bit fields
with 32-bit register names (eflags instead of rflags etc). Use the
guaranteed 32-bit underscore prefixed names for now where appropriate.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/cpu/vpmu.c
+++ b/xen/arch/x86/cpu/vpmu.c
@@ -265,7 +265,7 @@ void vpmu_do_interrupt(struct cpu_user_r
 cmp = (void *)&vpmu->xenpmu_data->pmu.r.regs;
 cmp->ip = cur_regs->rip;
 cmp->sp = cur_regs->rsp;
-cmp->flags = cur_regs->eflags;
+cmp->flags = cur_regs->rflags;
 cmp->ss = cur_regs->ss;
 cmp->cs = cur_regs->cs;
 if ( (cmp->cs & 3) > 1 )
@@ -288,7 +288,7 @@ void vpmu_do_interrupt(struct cpu_user_r
 
 r->ip = cur_regs->rip;
 r->sp = cur_regs->rsp;
-r->flags = cur_regs->eflags;
+r->flags = cur_regs->rflags;
 
 if ( !has_hvm_container_vcpu(sampled) )
 {
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1044,11 +1044,11 @@ int arch_set_info_guest(
 init_int80_direct_trap(v);
 
 /* IOPL privileges are virtualised. */
-v->arch.pv_vcpu.iopl = v->arch.user_regs.eflags & X86_EFLAGS_IOPL;
-v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
+v->arch.pv_vcpu.iopl = v->arch.user_regs._eflags & X86_EFLAGS_IOPL;
+v->arch.user_regs._eflags &= ~X86_EFLAGS_IOPL;
 
 /* Ensure real hardware interrupts are enabled. */
-v->arch.user_regs.eflags |= X86_EFLAGS_IF;
+v->arch.user_regs._eflags |= X86_EFLAGS_IF;
 
 if ( !v->is_initialised )
 {
@@ -2235,7 +2235,7 @@ void hypercall_cancel_continuation(void)
 else
 {
 if ( is_pv_vcpu(current) )
-regs->eip += 2; /* skip re-execute 'syscall' / 'int $xx' */
+regs->rip += 2; /* skip re-execute 'syscall' / 'int $xx' */
 else
 current->arch.hvm_vcpu.hcall_preempted = 0;
 }
@@ -2264,11 +2264,11 @@ unsigned long hypercall_create_continuat
 struct cpu_user_regs *regs = guest_cpu_user_regs();
 struct vcpu *curr = current;
 
-regs->eax = op;
+regs->rax = op;
 
 /* Ensure the hypercall trap instruction is re-executed. */
 if ( is_pv_vcpu(curr) )
-regs->eip -= 2;  /* re-execute 'syscall' / 'int $xx' */
+regs->rip -= 2;  /* re-execute 'syscall' / 'int $xx' */
 else
 curr->arch.hvm_vcpu.hcall_preempted = 1;
 
@@ -2297,12 +2297,12 @@ unsigned long hypercall_create_continuat
 arg = next_arg(p, args);
 switch ( i )
 {
-case 0: regs->ebx = arg; break;
-case 1: regs->ecx = arg; break;
-case 2: regs->edx = arg; break;
-case 3: regs->esi = arg; break;
-case 4: regs->edi = arg; break;
-case 5: regs->ebp = arg; break;
+case 0: regs->rbx = arg; break;
+case 1: regs->rcx = arg; break;
+case 2: regs->rdx = arg; break;
+case 3: regs->rsi = arg; break;
+case 4: regs->rdi = arg; break;
+case 5: regs->rbp = arg; break;
 }
 }
 }
@@ -2372,12 +2372,12 @@ int hypercall_xlat_continuation(unsigned
 
 switch ( i )
 {
-case 0: reg = ®s->ebx; break;
-case 1: reg = ®s->ecx; break;
-case 2: reg = ®s->edx; break;
-case 3: reg = ®s->esi; break;
-case 4: reg = ®s->edi; break;
-case 5: reg = ®s->ebp; break;
+case 0: reg = ®s->rbx; break;
+case 1: reg = ®s->rcx; break;
+case 2: reg = ®s->rdx; break;
+case 3: reg = ®s->rsi; break;
+case 4: reg = ®s->rdi; break;
+case 5: reg = ®s->rbp; break;
 default: BUG(); reg = NULL; break;
 }
 if ( (mask & 1) )
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -1584,10 +1584,10 @@ int __init construct_dom0(
 /*
  * Initial register values:
  *  DS,ES,FS,GS = FLAT_KERNEL_DS
- *   CS:EIP = FLAT_KERNEL_CS:start_pc
- *   SS:ESP = FLAT_KERNEL_SS:start_stack
- *  ESI = start_info
- *  [EAX,EBX,ECX,EDX,EDI,EBP are zero]
+ *   CS:rIP = FLAT_KERNEL_CS:start_pc
+ *   SS:rSP = FLAT_KERNEL_SS:start_stack
+ *  rSI = start_info
+ *  [rAX,rBX,rCX,rDX,rDI,rBP,R8-R15 are zero]
  */
 regs = &v->arch.user_regs;
 regs->ds = regs->es = regs->fs = regs->gs =
@@ -1596,10 +1596,10 @@ int __init construct_dom0(
 FLAT_KERNEL_SS : FLAT_COMPAT_KERNEL_SS);
 regs->cs = (!is_pv_32bit_domain(d) ?
 FLAT_KERNEL_CS : FLAT_COMPAT_KERNEL_CS);
-regs->eip = parms.virt_entry;
-regs->esp = vstack_end;
-regs->esi = vstartinfo_start;
-regs->eflags = X86_EFLAGS_IF;
+regs->rip = parms.virt

Re: [Xen-devel] [PATCH 03/10] x86/shadow: use unambiguous register names

[Tim Deegan]

At 03:38 -0700 on 20 Dec (1482205097), Jan Beulich wrote:
> This is in preparation of eliminating the mis-naming of 64-bit fields
> with 32-bit register names (eflags instead of rflags etc).
> 
> Signed-off-by: Jan Beulich 

Acked-by: Tim Deegan 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 04/13] pvh/acpi: Install handlers for ACPI-related PVH IO accesses

[Jan Beulich]

>>> On 17.12.16 at 00:18,  wrote:
> --- a/xen/include/public/arch-x86/hvm/save.h
> +++ b/xen/include/public/arch-x86/hvm/save.h
> @@ -527,7 +527,37 @@ DECLARE_HVM_SAVE_TYPE(HPET, 12, struct hvm_hw_hpet);
>  /*
>   * PM timer
>   */
> +#if __XEN_INTERFACE_VERSION__ >= 0x00040800
> +struct hvm_hw_pmtimer {
> +uint32_t tmr_val;   /* PM_TMR_BLK.TMR_VAL: 32bit free-running counter */
> +uint16_t pm1a_sts;  /* PM1a_EVT_BLK.PM1a_STS: status register */
> +uint16_t pm1a_en;   /* PM1a_EVT_BLK.PM1a_EN: enable register */
> +#if defined(__XEN__) || defined(__XEN_TOOLS__)
> +uint16_t gpe0_sts;
> +uint16_t gpe0_en;
> +#endif

Why inside another #ifdef? There's no other example in this file
which might have suggested to you that it needs doing this way.
In fact there are also no pre-existing uses of
__XEN_INTERFACE_VERSION__ in this header, so I also don't see
why you added one (and then with a slightly off value check).

If anything the _whole_ header would need to become Xen/tools
only.

> +static inline int _hvm_hw_fix_pmtimer(void *h, uint32_t size)
> +{
> +#if defined(__XEN__) || defined(__XEN_TOOLS__)
> +struct hvm_hw_pmtimer *acpi = (struct hvm_hw_pmtimer *)h;
> +
> +if ( size == sizeof(struct hvm_hw_pmtimer_compat) )
> +acpi->gpe0_sts = acpi->gpe0_en = 0;
> +#endif

Same here.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen/arm: fix rank/vgic locks inversion bug

[Julien Grall]

Hi Stefano,

On 20/12/2016 00:22, Stefano Stabellini wrote:

On Mon, 19 Dec 2016, Julien Grall wrote:

Hi Stefano,

On 19/12/2016 23:30, Stefano Stabellini wrote:

On Mon, 19 Dec 2016, Julien Grall wrote:

2) We run gic_update_one_lr and vgic_store_itargetsr in parallel
safely
and locklessly. There might be a way to do it, but it is not easy I
haven't found it yet.


Correct me if I am wrong. There are no restriction to write into
IROUTER/ITARGETSR while an IRQ is pending. So the irq_set_affinity could
be
called once at the beginning of vgic_irq_migrate.

We may receive the interrupt on the wrong physical CPU at the beginning.
But
it would be the same when writing into IROUTER/ITARGETSR.

This would remove the need to get the rank lock in gic_update_one_lr.

Did I miss anything?


I am not sure if we can do that: the virtual interrupt might not be
EOI'd yet at that time. The guest EOI is used to deactivate the
corresponding physical interrupt. Migrating the physical IRQ at that
time, could have unintended consequences? I am not sure what the spec
says about this, but even if it is correct on paper, I would prefer not
to put it to the test: I bet that not many interrupt controllers have
been heavily tested in this scenario. What do you think?


I don't think this is an issue. An interrupt could have the priority drop
happening on pCPU A and be deactivated on pCPU B if the vCPU A is been
migrated when the interrupt is inflight. So if it is fine here, why would not
it be when the guest is specifically requesting the routing?


That is true, but this is not exactly the same.


My example was to show you that an IRQ can have its priority dropped in 
pCPU A and been deactivated to pCPU B. Another example is when only the 
IRQ is been migrated. The spec does not promise you to receive the next 
interrupt on the CPU you asked because it may take time to update the 
GIC state. So the priority drop and deactivation could be done on 
separate physical CPU here too.


> This is changing the

physical irq affinity while both physical and virtual irqs are still
active.


Physical IRQ state and virtual IRQ state are completely dissociated in 
the GIC. The only interaction possible is the virtual interface to send 
a deactivate request to the distributor when the virtual interrupt has 
been deactivated and correspond to a hardware interrupt.



As I wrote, usually operating systems only change affinity after
deactivating an irq, so I thought it would be wise in Xen to wait at
least for the EOI.


I looked at the Linux code and did not see a such requirement when 
setting the affinity (see irq_set_affinity) of an IRQ.



If we tried to inject the same virtual interrupt on a
different vcpu, while the interrupt is still inflight, we could get in
troubles. But we could avoid that by testing for GIC_IRQ_GUEST_MIGRATING
in vgic_vcpu_inject_irq. Maybe I am worrying about nothing.


The only interrupt that can be routed to a guest in Xen are SPI which 
are shared between all CPUs. The active bit is handled by the 
distributor. It is not possible to receive the same SPI until it has 
been deactivated.


Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Ping: [PATCH 0/3] x86emul: misc adjustments

[Andrew Cooper]

On 20/12/2016 09:04, Jan Beulich wrote:
 On 12.12.16 at 10:38,  wrote:
>> These patches are grouped together merely because of contextual
>> dependencies.
>>
>> 1: correct EFLAGS.TF handling
>> 2: conditionally clear BNDn for branches
>> 3: some REX related polishing
>>
>> Signed-off-by: Jan Beulich 
> Any word on these? I realize patch 1 will need to be re-based over
> yesterday's XSA-204 patch, and as mentioned elsewhere may also
> want to suppress setting of retire.sti when TF=1 and BTF=0, but
> before sending a v2 of that I'd prefer knowing whether further
> adjustments may be needed.

I'd like to get a better understanding of the interaction of shadows
before passing review on patch 1.  I don't have any specific issues with
it though.

The repeated reads of MSR_DEBUGCTL are a little concerning, but your
proposed alternate for vendor_is() has given me an idea how to fix this
once the MSR levelling work starts, so I am less concerned.

I also need to read up on MPX to review patch 2.

Patch 3 on the other hand looks fine.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 3/3] x86emul: some REX related polishing

[Andrew Cooper]

On 12/12/2016 10:00, Jan Beulich wrote:
> While there are a few cases where it seems better to open-code REX_*
> values, there's one where this clearly is a bad idea. And the SYSEXIT
> emulation has no need to look at REX at all, it can simply use op_bytes
> instead.
>
> Signed-off-by: Jan Beulich 

Reviewed-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [ovmf test] 103763: all pass - PUSHED

[osstest service owner]

flight 103763 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/103763/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 83c6c3bfe293cd0a5983375dfbf46d4f364b38aa
baseline version:
 ovmf 15dae68589243726f0374e535a77e76444096bad

Last test of basis   103748  2016-12-19 11:44:06 Z0 days
Testing same since   103763  2016-12-20 01:49:49 Z0 days1 attempts


People who touched revisions under test:
  Yonghong Zhu 

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 :

+ branch=ovmf
+ revision=83c6c3bfe293cd0a5983375dfbf46d4f364b38aa
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push ovmf 
83c6c3bfe293cd0a5983375dfbf46d4f364b38aa
+ branch=ovmf
+ revision=83c6c3bfe293cd0a5983375dfbf46d4f364b38aa
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . ./cri-common
++ . ./cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=ovmf
+ xenbranch=xen-unstable
+ '[' xovmf = xlinux ']'
+ linuxbranch=
+ '[' x = x ']'
+ qemuubranch=qemu-upstream-unstable
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable
+ prevxenbranch=xen-4.8-testing
+ '[' x83c6c3bfe293cd0a5983375dfbf46d4f364b38aa = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/qemu-xen-traditional.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/xtf.git
++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git
++ : git://xenbits.xen.org/xtf.git
++ : git://xenbits.xen.org/libvirt.git
++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git
++ : git://xenbits.xen.org/libvirt.git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/rumprun.git
++ : git://git.seabios.org/seabios.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/seabios.git
++ : git://xenbits.xen.org/osstest/seabios.git
++ : https://github.com/tianocore/edk2.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/linux-firmware.git
++ : osst...@xenbits.xen.org:/home/osstest/ext/linux-firmware.git
++ : git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git
++ : osst...@xenbits.xen.org:/home/xen/git/linux-pvops

Re: [Xen-devel] Xen: ARM: Support for mapping ECAM PCIe Config Space Specified In Static ACPI Table

[Julien Grall]

Hi Jiandi,

On 20/12/2016 07:31, Jiandi An wrote:

On 12/19/16 07:11, Julien Grall wrote:



On 19/12/2016 13:20, Jaggi, Manish wrote:

On 16/12/2016 15:49, Julien Grall wrote:

On 14/12/16 08:00, Jiandi An wrote:

Xen currently doesn't map ECAM space specified in static ACPI table.
Seeking opinion on how this should be handled properly.
Each root complex ECAM region takes up 64K 4K pages (256MB).
For some platforms there might be multiple root complexes.
Is the plan to map all at once?Julien has mentioned support
for mapping ECAM may come when support for PCI passthrough is
added, is that right? What mechanism will it be? Will Xen or
dom0 be the one that parses the staic ACPI tables and map the ECAM space?


For performance reason, each ECAM region would need to be mapped at
once, so the stage-2 page table could take advantage of superpage (it
will mostly be 2MB).

Now, I don't think Xen should map the ECAM region in stage-2 before
hand. All the regions may not be described in the MCFG and I would like
to see a generic solution.

Looking at the code (see pci_create_ecam_create in drivers/pci/ecam.c),
ioremap is used. I believe the problem is the same for the 2 other
threads you sent ( [1] and [2]).

So it might be good to look at hooking up a call to
XENMEM_add_to_physmap_range in ioremap.

Any opinions?


I thought a bit more about it and I realized we need to be cautious on
how to proceed here.

DOM0 will have a mix of real devices and emulated devices (e.g some part
of the GIC). For the emulated devices, DOM0 should not call
XENMEM_add_to_physmap_range. However, DOM0 is not aware what is emulated
or not, so even the current approach (hooking up in platform device)
seems fragile. We rely on Xen to say "this region cannot be mapped".


 Why not add support for parsing ACPI tables in Xen, from linux,  as we parse 
dt.


Because MMIO can be described in ASL too. I would rather avoid to have a 
different behavior depending whether the MMIO has been described in static 
table or ASL.

Cheers,



I also think hooking up a call to XENMEM_add_to_physmap_range in ioremap
is not a good approach as ioremap() is commonly called in so many places.
It's not ideal to make a check of am I dom0 running under xen every time
ioremap() is called.  And Julien also pointed out, not every call to ioremap()
needs a stage 2 mapping.


I think you misunderstood my previous e-mail. Xen cannot differentiate 
whether an MMIO region is being emulated. So if Xen decides to emulate 
an AMBA device, we would be in the same trouble.


To be clear, in my previous mail I was pointing a drawback of this 
solution. But I believe this is the best way to get the stage-2 mapping 
correct and limiting the size of stage-2 PT for DOM0.


Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 05/13] pvh/acpi: Handle ACPI accesses for PVH guests

[Jan Beulich]

>>> On 17.12.16 at 00:18,  wrote:
> +static int acpi_cpumap_access_common(struct domain *d,
> + int dir, unsigned int port,
> + unsigned int bytes, uint32_t *val)
> +{
> +unsigned int first_byte = port - XEN_ACPI_CPU_MAP;
> +
> +BUILD_BUG_ON(XEN_ACPI_CPU_MAP + XEN_ACPI_CPU_MAP_LEN
> + >= ACPI_GPE0_BLK_ADDRESS_V1);

Just > afaict.

> +
> +if ( dir == XEN_DOMCTL_ACPI_READ )
> +{
> +uint32_t mask = (bytes < 4) ? ~0U << (bytes * 8) : 0;
> +
> +/*
> + * Clear bits that we are about to read to in case we
> + * copy fewer than @bytes.
> + */
> +*val &= mask;
> +
> +if ( ((d->max_vcpus + 7) / 8) > first_byte )
> +memcpy(val, (uint8_t *)d->avail_vcpus + first_byte,
> +   min(bytes, ((d->max_vcpus + 7) / 8) - first_byte));
> +}
> +else
> +/* Guests do not write CPU map */
> +return X86EMUL_UNHANDLEABLE;

Perhaps make this an early bail, reducing overall indentation?

> +static int acpi_access_common(struct domain *d,
> +  int dir, unsigned int port,
> +  unsigned int bytes, uint32_t *val)
> +{
> +uint16_t *sts = NULL, *en = NULL;
> +const uint16_t *mask_sts = NULL, *mask_en = NULL;
> +static const uint16_t pm1a_sts_mask = ACPI_BITMASK_GLOBAL_LOCK_STATUS;
> +static const uint16_t pm1a_en_mask = ACPI_BITMASK_GLOBAL_LOCK_ENABLE;
> +static const uint16_t gpe0_sts_mask = 1U << XEN_ACPI_GPE0_CPUHP_BIT;
> +static const uint16_t gpe0_en_mask = 1U << XEN_ACPI_GPE0_CPUHP_BIT;
> +
> +ASSERT(!has_acpi_dm_ff(d));
> +
> +switch ( port )
> +{
> +case ACPI_PM1A_EVT_BLK_ADDRESS_V1 ...
> +ACPI_PM1A_EVT_BLK_ADDRESS_V1 +
> +sizeof(d->arch.hvm_domain.acpi.pm1a_sts) +
> +sizeof(d->arch.hvm_domain.acpi.pm1a_en):
> +
> +sts = &d->arch.hvm_domain.acpi.pm1a_sts;
> +en = &d->arch.hvm_domain.acpi.pm1a_en;
> +mask_sts = &pm1a_sts_mask;
> +mask_en = &pm1a_en_mask;
> +break;
> +
> +case ACPI_GPE0_BLK_ADDRESS_V1 ...
> +ACPI_GPE0_BLK_ADDRESS_V1 +
> +sizeof(d->arch.hvm_domain.acpi.gpe0_sts) +
> +sizeof(d->arch.hvm_domain.acpi.gpe0_en):
> +
> +sts = &d->arch.hvm_domain.acpi.gpe0_sts;
> +en = &d->arch.hvm_domain.acpi.gpe0_en;
> +mask_sts = &gpe0_sts_mask;
> +mask_en = &gpe0_en_mask;
> +break;
> +
> +default:
> +return X86EMUL_UNHANDLEABLE;
> +}
> +
> +if ( dir == XEN_DOMCTL_ACPI_READ )
> +{
> +uint32_t mask = (bytes < 4) ? ~0U << (bytes * 8) : 0;
> +uint32_t data = (((uint32_t)*en) << 16) | *sts;

There's one pair of pointless parentheses around the cast
expression here.

> +
> +data >>= 8 * (port & 3);
> +*val = (*val & mask) | (data & ~mask);
> +}
> +else
> +{
> +uint32_t v = *val;
> +
> +/* Status register is write-1-to-clear by guests */
> +switch ( port & 3 )
> +{
> +case 0:
> +*sts &= ~(v & 0xff);
> +*sts &= *mask_sts;

I can understand the first &=, but why would a read have this second
(side) effect? I could see some sort of need for such only when you
were setting any flags.

> +if ( !--bytes )
> +break;
> +v >>= 8;
> +/* fallthrough */
> +case 1:
> +*sts &= ~((v & 0xff) << 8);
> +*sts &= *mask_sts;
> +if ( !--bytes )
> +break;
> +v >>= 8;
> +/* fallthrough */
> +case 2:
> +*en = ((*en & 0xff00) | (v & 0xff)) & *mask_en;
> +if ( !--bytes )
> +break;
> +v >>= 8;
> +/* fallthrough */
> +case 3:
> +*en = (((v & 0xff) << 8) | (*en & 0xff)) & *mask_en;
> +break;
> +}
> +}
> +
> +return X86EMUL_OKAY;
> +}
> +
> +
>  int hvm_acpi_domctl_access(struct domain *d, uint8_t rw,

No double blank lines please.

> @@ -18,13 +135,19 @@ int hvm_acpi_domctl_access(struct domain *d, uint8_t rw,
>  static int acpi_guest_access(int dir, unsigned int port,
>   unsigned int bytes, uint32_t *val)
>  {
> -return X86EMUL_UNHANDLEABLE;
> +return  acpi_access_common(current->domain,
> +   (dir == IOREQ_READ) ?
> +   XEN_DOMCTL_ACPI_READ: XEN_DOMCTL_ACPI_WRITE,
> +   port, bytes, val);
>  }

I don't think this one is meant to be used by the domctl, so I don't see
why you need a helper here. That would also eliminate the seemingly
unnecessary use of XEN_DOMCTL_* here (I think you already had said
this was an oversight in an earlier reply).

Jan

___
Xen-devel mailing list
Xen-devel@lists.xen.org
htt

Re: [Xen-devel] Ping: [PATCH 0/3] x86emul: misc adjustments

[Jan Beulich]

>>> On 20.12.16 at 12:30,  wrote:
> On 20/12/2016 09:04, Jan Beulich wrote:
> On 12.12.16 at 10:38,  wrote:
>>> These patches are grouped together merely because of contextual
>>> dependencies.
>>>
>>> 1: correct EFLAGS.TF handling
>>> 2: conditionally clear BNDn for branches
>>> 3: some REX related polishing
>>>
>>> Signed-off-by: Jan Beulich 
>> Any word on these? I realize patch 1 will need to be re-based over
>> yesterday's XSA-204 patch, and as mentioned elsewhere may also
>> want to suppress setting of retire.sti when TF=1 and BTF=0, but
>> before sending a v2 of that I'd prefer knowing whether further
>> adjustments may be needed.
> 
> I'd like to get a better understanding of the interaction of shadows
> before passing review on patch 1.  I don't have any specific issues with
> it though.
> 
> The repeated reads of MSR_DEBUGCTL are a little concerning, but your
> proposed alternate for vendor_is() has given me an idea how to fix this
> once the MSR levelling work starts, so I am less concerned.

After re-basing there are no such repeated reads anymore afaict:
The MSR will be read once when TF is set (at the time where the
singlestep local variable get initialized), and then only again in
put_loop_count() (just like in v1). I'll send out v2 later.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86emul: don't unconditionally clear segment bases upon null selector loads

[Andrew Cooper]

On 20/12/2016 08:18, Jan Beulich wrote:
> AMD explicitly documents that namely FS and GS don't have their bases
> cleared in that case, and I see no reason why guests may not rely on
> that behavior. To facilitate this a new input field (the CPU vendor) is
> being added.
>
> Signed-off-by: Jan Beulich 

This looks better overall.

Longterm I think it would be better to pass the full cpuid policy in to
the emulator. This removes the need to use the cpuid() hook for both
emulation and instruction related purposes, which we have seen gets
complicated with CPUID Faulting handling.  Looking further than that,
passing the full MSR banks would simplify that side of things as well.

Reviewed-by: Andrew Cooper , with one minor
correction

> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -1897,6 +1897,7 @@ void hvm_emulate_init_once(
>  hvmemul_get_seg_reg(x86_seg_ss, hvmemul_ctxt);
>  
>  hvmemul_ctxt->ctxt.regs = regs;
> +hvmemul_ctxt->ctxt.vendor = current->domain->arch.x86_vendor;

curr is available here.

~Andrew

>  hvmemul_ctxt->ctxt.force_writeback = true;
>  
>  if ( cpu_has_vmx )
>


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] merlot0 and merlot1 (was Re: [xen-4.4-testing test] 103756: trouble: blocked/broken/fail/pass)

[Ian Jackson]

osstest service owner writes ("[xen-4.4-testing test] 103756: trouble: 
blocked/broken/fail/pass"):
> flight 103756 xen-4.4-testing real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/103756/
> 
> Failures and problems with tests :-(
> 
> Tests which did not succeed and are blocking,
> including tests which could not be run:
>  build-i386-xend3 host-install(3)broken REGR. vs. 103438

merlot1 had lost its BIOS settings including boot order.  While
repairing this I found that merlot0 had too.  I wonder if that might
be a software-induced problem.

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Xen: ARM: Support for mapping ECAM PCIe Config Space Specified In Static ACPI Table

[Julien Grall]

Hi Stefano,

On 20/12/2016 00:54, Stefano Stabellini wrote:

On Mon, 19 Dec 2016, Julien Grall wrote:

On 16/12/2016 15:49, Julien Grall wrote:

On 14/12/16 08:00, Jiandi An wrote:

Xen currently doesn't map ECAM space specified in static ACPI table.
Seeking opinion on how this should be handled properly.
Each root complex ECAM region takes up 64K 4K pages (256MB).
For some platforms there might be multiple root complexes.
Is the plan to map all at once?Julien has mentioned support
for mapping ECAM may come when support for PCI passthrough is
added, is that right? What mechanism will it be? Will Xen or
dom0 be the one that parses the staic ACPI tables and map the ECAM space?


For performance reason, each ECAM region would need to be mapped at
once, so the stage-2 page table could take advantage of superpage (it
will mostly be 2MB).

Now, I don't think Xen should map the ECAM region in stage-2 before
hand. All the regions may not be described in the MCFG and I would like
to see a generic solution.

Looking at the code (see pci_create_ecam_create in drivers/pci/ecam.c),
ioremap is used. I believe the problem is the same for the 2 other
threads you sent ( [1] and [2]).

So it might be good to look at hooking up a call to
XENMEM_add_to_physmap_range in ioremap.

Any opinions?


I thought a bit more about it and I realized we need to be cautious on how to
proceed here.

DOM0 will have a mix of real devices and emulated devices (e.g some part of
the GIC). For the emulated devices, DOM0 should not call
XENMEM_add_to_physmap_range. However, DOM0 is not aware what is emulated or
not, so even the current approach (hooking up in platform device) seems
fragile. We rely on Xen to say "this region cannot be mapped".


You are right that Dom0 doesn't and shouldn't be able to tell the
difference between emulated and real devices. But I don't think that
should be a problem because Xen knows exactly if an MMIO region belongs
to an emulated device thanks to the 1:1 mapping. When
XENMEM_add_to_physmap_range is called, Xen can check whether the region
belongs to an emulated device or a real device, mapping memory only if
it belongs to a real device. No need to report errors: from Dom0 point
of view the operation succeeded either way.


By no need to report errors, did you mean Xen failing, or DOM0 failing?

I looked at the code (map_dev_mmio_region), we check whether DOM0 is 
allowed to access the iomem. If a part of the region is not accessible, 
it will map nothing and return as it has succeeded.


This behavior is fairly odd because it means that a domain will never 
know whether a region has been mapped correctly. It may fail later on 
because, for instance, the domain was trying to map the device with MFN 
!= GFN. Thankfully we only map one page at the time (see the caller 
xenmem_add_to_physmap_one) but still a domain will expect to know what's 
going on.


So I think we need to associate a specific errno to tell the domain this 
region is not accessible.


Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 2/2] x86/emul: Bugfixes to SYSCALL emulation

[Andrew Cooper]

On 20/12/2016 08:22, Jan Beulich wrote:
 On 19.12.16 at 17:37,  wrote:
>> Introduce vendor_is() to allow emulation to have vendor-specific
>> behaviour.  Adjust the SYSCALL behaviour on Intel to raise #UD when
>> executed outside of 64bit mode.
> I'd rather not see us go this route. I've been carrying a patch
> making the vendor an input (not submitted so far due to other
> at least contextual prereqs missing when I last check, but sent
> out just a minute ago). What do you think?

Rebasing on top of that would be far more simple.

>
>> in_longmode() has different return semantics from rc, so use a separate
>> integer for the purpose.
> The ugliness of the additional #ifdef doesn't seem to warrant
> this.

It is a matter of correctness.  The only reason we exit from it cleanly
is because the cannot_emulate path discards rc, while the other paths
overwrite rc because of hitting the read_msr() or write_segment() hooks
in each case.

> What I've been thinking the other day though is: Why
> don't we put the whole SYSCALL emulation into a __XEN__
> conditional (implying __x86_64__, i.e. allowing the inner ones
> to be removed)?

This depends on whether we think it is ever realistic to be able to be
able to test things like this in the userspace harness.  I am not sure
we realistically can.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86/time: Adjust init-time handling of pit0_ticks

[Andrew Cooper]

On 20/12/2016 07:25, Jan Beulich wrote:
 On 19.12.16 at 17:58,  wrote:
>> On 19/12/16 16:51, Jan Beulich wrote:
>> On 19.12.16 at 17:38,  wrote:
 There is no need for the volatile cast in the timer interrupt.  pit0_ticks 
 has
 external linkage, preventing the compiler from eliding the update.  This
 reduces the generated assembly from a read, local modify, write to a single
 add instruction.
>>> I don't think external linkage is the reason here, considering the
>>> effects of whole-program-optimization.
>> In the case of whole-program-optimisation, the compiler would observe
>> that one function wrote to the variable, and one function read from it. 
>> I presume that is also sufficient to prevent the eliding?
> I would think so, yes (albeit the end result of that process may
> be that everything which isn't recursive and doesn't serve as
> independent entry point ends up as a few huge functions); I
> merely wanted to point out that linkage isn't really relevant here.

What about this?

There is no need for the volatile cast in the timer interrupt; the compiler
may not elide the update.  This reduces the generated assembly from a read,
local modify, write to a single add instruction.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen/x86: Improve hypercall page writing

[Andrew Cooper]

On 20/12/2016 07:48, Jan Beulich wrote:
>
>> @@ -398,10 +400,11 @@ static void 
>> hypercall_page_initialise_ring1_kernel(void *hypercall_page)
>>   * calling it.
>>   */
>>  p = (char *)(hypercall_page + (__HYPERVISOR_iret * 32));
>> -*(u8  *)(p+ 0) = 0x50;/* push %eax */
>> -*(u8  *)(p+ 1) = 0xb8;/* mov  $__HYPERVISOR_iret,%eax */
>> -*(u32 *)(p+ 2) = __HYPERVISOR_iret;
>> -*(u16 *)(p+ 6) = (HYPERCALL_VECTOR << 8) | 0xcd; /* int  $xx */
>> +memcpy(p,
>> +   "\x50" /* push %eax */
>> +   "\xb8\x17\x00\x00\x00" /* mov  $__HYPERVISOR_iret, %eax */
>> +   "\xcd\x82",/* int  $HYPERCALL_VECTOR */
> here as a good idea.

Well, they are fixed in the ABI.  It is not as if we could ever change them.

>  If you used a static const uint8_t[] instead of
> a string literal (which even includes a pointless nul terminator), all of
> this could be avoided afaict.

I can see how that would work for the `int $0x82` case, but how do you
propose fitting 4 bytes of __HYPERVISOR_iret into an initialiser for a
uint8_t array?

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Xen ARM - Exposing a PL011 to the guest

[Julien Grall]

Hi Stefano,

On 19/12/2016 21:24, Stefano Stabellini wrote:

On Mon, 19 Dec 2016, Christoffer Dall wrote:

On Fri, Dec 16, 2016 at 05:03:13PM +, Julien Grall wrote:

(CC rest maintainers for event channel questions)

On 16/12/16 10:06, Bhupinder Thakur wrote:

Hi,


Hi Bhupinder,


The idea is for Xen to act as an intermediary as shown below:

  ring buffers
  rx/tx fifo
dom0 <---> Xen HYP (running pl011 emulation)
<---> domU
   event
  interrupts

Xen will directly manage the in/out console ring buffers (allocated by
dom0 for dom0-domU console communication) for reading/writing console
data from/to dom0. On the other side, Xen HYP will emulate pl011 to
read/write data from/to domU and pass it on to/from dom0 over the
in/out console ring buffers. There should be no change in dom0 as it
will still use the same ring buffers. Similarly there should be no
change in domU which would be running a standard pll011 driver.

Currently, I am working on the interface between dom0 and Xen HYP. I
want to intercept the console events in Xen HYP which pass between
dom0 and domU. For now, I just want to capture console data coming

>from dom0 at Xen HYP and loop it back to dom0, to confirm that this

interface is working.

Since each guest domain will have a unique event channel assigned for
console communication, Xen HYP can find out the event channel for a
given domU from the start_info page of that domU, which should have


The start_info page is x86 specific. If you want to get the console
event channel for ARM, you would have to use
d->arch.hvm_domain.params[HVM_PARAM_CONSOLE_EVTCHN].

This parameter will be setup by the toolstack (see alloc_magic_pages
in libxc/xc_dom_arm.c).


been allocated by dom0. Whenever, an event is to be dispatched via
evtchn_send() API in Xen, it can check if the event channel is the
console event channel for a given domU. If yes and its source domain
is dom0 and destination domain is domU then it will write the data
back to the console out ring buffer of the domU and raise a console
event to dom0.

Once this interface is working, Xen HYP can check the source and
destination dom ids and decide which way the event came from and
accordingly process the console data. To allow a mix of PV console
guests and pl011 guests, Xen might have to maintain a flag per domain,
which tells whether Xen HYP should intercept and process the data (for
pl011 UART case) or let it go transparently (for PV conosle case).


I am not very familiar with the event channel code. I will let the
others comment on this bit.

Regardless that, how would you decide whether the hypervisor should
intercept the notification?

I can see 2 different cases:
1) The guest is starting to use the pl011 then move to the HVC
console (or HVC then pl011)
2) The guest is using both the PL011 and the HVC console

Should we consider the second case valid? I would say yes, because a
user could specify both on the command line. If we use the same
ring, the output would be a total garbage.

So maybe we need to allocate two distinct rings and event channel?


This sounds like the only sensible thing to me.  I think this is really
about adding a new device to the Xen virtual platform, and providing the
user the option to choose which one he wants the tool in Dom0 to be
presented using stdin/out. Presumably the other console/serial can be
redirected to a file or socket or something?


Let me explain how the PV console protocol and drivers work, because
they are a bit unusual. The first PV console is advertised via
hvm_params. The guest calls:

  hvm_get_parameter(HVM_PARAM_CONSOLE_EVTCHN, &v);
  hvm_get_parameter(HVM_PARAM_CONSOLE_PFN, &v);

to get the two parameters to setup the ring and evtchn. If they are 0,
the guest considers the first console unavailable. Other PV console
rings, from the second onward, are advertised via xenstore like any
other Xen PV protocols. In those cases, frontend and backend access
xenstore to setup ring and event channel.

The PV console backends are unusual too. xenconsoled, available on all
Xen systems, is one process per host and can handle only one PV console
per domain. Specifically, it is only able to deal with the first console.
Domains that have multiple PV consoles require QEMU (not as an emulator,
but as a PV backends provider). The toolstack writes "type" =
"xenconsoled" or "ioemu" to distinguish PV consoles that xenconsoled or
QEMU are supposed to handle. Ideally, we shouldn't require QEMU for
pl011 PV consoles, but it wouldn't be the end of the world if we did.

Additionally, Xen cannot speak xenstore. It can neither read nor write
to it. I don't think we should add xenstore support to the hypervisor
for this. We need to come up with a solution that doesn't require it.


Agree on this.



Finally, we cannot hijack one of the guest PV consoles, regardless of
whether it's the firs

Re: [Xen-devel] Ping: [PATCH v2 RESEND] x86/time: correctly honor late clearing of TSC related feature flags

[Andrew Cooper]

On 15/12/2016 15:09, Jan Beulich wrote:
 On 15.12.16 at 12:04,  wrote:
>> On 15/12/16 09:49, Jan Beulich wrote:
>> On 06.12.16 at 11:51,  wrote:
 As such clearing of flags may have an impact on the selected rendezvous
 function, handle such in a central place.

 But don't allow such feature flags to be cleared during CPU hotplug:
 Platform and local system times may have diverged significantly by
 then, potentially causing noticably (even if only temporary) strange
 behavior. As we're anyway expecting only sufficiently similar CPUs to
 appear during hotplug, this shouldn't be introducing new limitations.

 Reported-by: Joao Martins 
 Signed-off-by: Jan Beulich 
 Tested-by: Dario Faggioli 
 Tested-by: Joao Martins 
 ---
 The resend is mainly to get the discussion going again on what the
 alternatives are, if this patch is not acceptable.
>>> Even if you don't agree with the patch, can we at least revive
>>> the discussion of what alternatives there are?
>> Sorry - it slipped through the cracks.  I have no issue with the
>> principle of the patch.
>>
>> The only problem I have, which we didn't sort out last time, is the
>> initialisation of rendezvous_fn
>>
>> It is still my opinion that under no circumstance is it ok for
>> clear_tsc_cap() to modify time_calibration_rendezvous_fn from
>> time_calibration_tsc_rendezvous to time_calibration_std_rendezvous,
>> which can in principle happen because rendezvous_fn doesn't get
>> initialised from the current time_calibration_rendezvous_fn.
> I understand that this is your primary reservation against the patch,
> yet at the same time this is also the purpose of the patch. If we're
> not allowed to change the rendezvous function to what it is supposed
> to be given the final set of CPU features we determined, then the
> whole patch is pointless. At the time we first set the pointer, we
> simply don't know yet what we'll find once we brought up APs. If we
> knew, we'd set it differently. Since pre- and post-AP bringup
> knowledge will always have the potential to differ, we need to adjust
> the function pointer in one direction anyway: Either we set it to std
> early, and switch to tsc later, or we allow setting it to tsc early,
> reverting to std if need be.

Once we have ever had cause to use time_calibration_tsc_rendezvous,
there is no situation where it is safe to switch back to
time_calibration_std_rendezvous, as the choice to use
time_calibration_tsc_rendezvous means the TSCs aren't in sync, and Xen
has no practical mechanism to resync them.  (We could guarantee not to
ever invoke Cstates, including C1E, but this doesn't prevent an SMI
doing that for us.)

time_calibration_rendezvous_fn should start with the best case scenario,
and be gradually made worse by our AP discovery, but should never get
better.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 0/1] xen/arm: Map mmio-sram nodes as cached memory

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

This patch changes the mapping from non-cached to cached
for mmio-sram nodes that do not have the no-memory-wc property.
This is a hang-over from 4.8 since the mmio-sram patches went
in late in the cycle.

I've explained the rationale in the commit message:
Although on chip memories are relatively fast compared to
off-chip memories, large on chip memories are still
significantly slower than L1 caches. Depending on the
memory, either cached or uncached may make most sense.

Also, hardware domains may like to use the memory in a
cache-coherent way to avoid SW managed coherency.

By mapping it cached at S2, we let hardware domains select
cacheability via S1 mappings.

This was also discussed here:
https://lists.xenproject.org/archives/html/xen-devel/2016-05/msg02412.html

Were we reached the conclusion that we should be using the most relaxed
attributes as long as we don't compromise security.

Since mmio-sram DTS noders have a property (no-memory-wc) to describe
srams with access restrictions, we can select the most relaxed mode
for unrestricted ones.

Cheers,
Edgar

ChangeLog:

v1 -> v2:
* Spell out On Chip Memories (OCMs)
* Avoid dom0 in preference of hardware domains
* Fix spelling of although

Edgar E. Iglesias (1):
  xen/arm: Map mmio-sram nodes as cached memory

 xen/arch/arm/domain_build.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 1/1] xen/arm: Map mmio-sram nodes as cached memory

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Relax the mapping of mmio-sram nodes that do not set the
no-memory-wc property to cached normal memory.

Rationale:
Although on chip memories are relatively fast compared to
off-chip memories, large on chip memories are still
significantly slower than L1 caches. Depending on the
memory, either cached or uncached may make most sense.

Also, hardware domains may like to use the memory in a
cache-coherent way to avoid SW managed coherency.

By mapping it cached at S2, we let hardware domains select
cacheability via S1 mappings.

Signed-off-by: Edgar E. Iglesias 
---
 xen/arch/arm/domain_build.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 07b868d..08646b8 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -56,8 +56,20 @@ static const struct dt_device_match dev_map_attrs[] 
__initconst =
 .data = (void *) (uintptr_t) p2m_mmio_direct_dev,
 },
 {
+/*
+ * Although on chip memories are relatively fast compared to
+ * off-chip memories, large on chip memories are still
+ * significantly slower than L1 caches. Depending on the
+ * memory, either cached or uncached may make most sense.
+ *
+ * Also, hardware domains may like to use the memory in a
+ * cache-coherent way to avoid SW managed coherency.
+ *
+ * By mapping it cached at S2, we let hardware domains select
+ * cacheability via S1 mappings.
+ */
 __DT_MATCH_COMPATIBLE("mmio-sram"),
-.data = (void *) (uintptr_t) p2m_mmio_direct_nc,
+.data = (void *) (uintptr_t) p2m_mmio_direct_c,
 },
 { /* sentinel */ },
 };
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Xen ARM - Exposing a PL011 to the guest

[Christoffer Dall]

Hi Stefano,

On Mon, Dec 19, 2016 at 12:24:18PM -0800, Stefano Stabellini wrote:
> On Mon, 19 Dec 2016, Christoffer Dall wrote:
> > On Fri, Dec 16, 2016 at 05:03:13PM +, Julien Grall wrote:
> > > (CC rest maintainers for event channel questions)
> > > 
> > > On 16/12/16 10:06, Bhupinder Thakur wrote:
> > > >Hi,
> > > 
> > > Hi Bhupinder,
> > > 
> > > >The idea is for Xen to act as an intermediary as shown below:
> > > >
> > > >   ring buffers
> > > >   rx/tx fifo
> > > >dom0 <---> Xen HYP (running pl011 emulation)
> > > ><---> domU
> > > >event
> > > >   interrupts
> > > >
> > > >Xen will directly manage the in/out console ring buffers (allocated by
> > > >dom0 for dom0-domU console communication) for reading/writing console
> > > >data from/to dom0. On the other side, Xen HYP will emulate pl011 to
> > > >read/write data from/to domU and pass it on to/from dom0 over the
> > > >in/out console ring buffers. There should be no change in dom0 as it
> > > >will still use the same ring buffers. Similarly there should be no
> > > >change in domU which would be running a standard pll011 driver.
> > > >
> > > >Currently, I am working on the interface between dom0 and Xen HYP. I
> > > >want to intercept the console events in Xen HYP which pass between
> > > >dom0 and domU. For now, I just want to capture console data coming
> > > >from dom0 at Xen HYP and loop it back to dom0, to confirm that this
> > > >interface is working.
> > > >
> > > >Since each guest domain will have a unique event channel assigned for
> > > >console communication, Xen HYP can find out the event channel for a
> > > >given domU from the start_info page of that domU, which should have
> > > 
> > > The start_info page is x86 specific. If you want to get the console
> > > event channel for ARM, you would have to use
> > > d->arch.hvm_domain.params[HVM_PARAM_CONSOLE_EVTCHN].
> > > 
> > > This parameter will be setup by the toolstack (see alloc_magic_pages
> > > in libxc/xc_dom_arm.c).
> > > 
> > > >been allocated by dom0. Whenever, an event is to be dispatched via
> > > >evtchn_send() API in Xen, it can check if the event channel is the
> > > >console event channel for a given domU. If yes and its source domain
> > > >is dom0 and destination domain is domU then it will write the data
> > > >back to the console out ring buffer of the domU and raise a console
> > > >event to dom0.
> > > >
> > > >Once this interface is working, Xen HYP can check the source and
> > > >destination dom ids and decide which way the event came from and
> > > >accordingly process the console data. To allow a mix of PV console
> > > >guests and pl011 guests, Xen might have to maintain a flag per domain,
> > > >which tells whether Xen HYP should intercept and process the data (for
> > > >pl011 UART case) or let it go transparently (for PV conosle case).
> > > 
> > > I am not very familiar with the event channel code. I will let the
> > > others comment on this bit.
> > > 
> > > Regardless that, how would you decide whether the hypervisor should
> > > intercept the notification?
> > > 
> > > I can see 2 different cases:
> > >   1) The guest is starting to use the pl011 then move to the HVC
> > > console (or HVC then pl011)
> > >   2) The guest is using both the PL011 and the HVC console
> > > 
> > > Should we consider the second case valid? I would say yes, because a
> > > user could specify both on the command line. If we use the same
> > > ring, the output would be a total garbage.
> > > 
> > > So maybe we need to allocate two distinct rings and event channel?
> > 
> > This sounds like the only sensible thing to me.  I think this is really
> > about adding a new device to the Xen virtual platform, and providing the
> > user the option to choose which one he wants the tool in Dom0 to be
> > presented using stdin/out. Presumably the other console/serial can be
> > redirected to a file or socket or something?
> 
> Let me explain how the PV console protocol and drivers work, because
> they are a bit unusual. 

Thanks for this.  As my detailed knowledge of Xen is relatively limited
I can only contribute with a very high level approach to which problem
we're trying to solve.


> The first PV console is advertised via
> hvm_params. The guest calls:
> 
>   hvm_get_parameter(HVM_PARAM_CONSOLE_EVTCHN, &v);
>   hvm_get_parameter(HVM_PARAM_CONSOLE_PFN, &v);
> 
> to get the two parameters to setup the ring and evtchn. If they are 0,
> the guest considers the first console unavailable. Other PV console
> rings, from the second onward, are advertised via xenstore like any
> other Xen PV protocols. In those cases, frontend and backend access
> xenstore to setup ring and event channel.

So, hvm_get_parameter does not access xenstore, because you need the
console because xenstore is available, but subsequent consoles can just
access xenstore, or did I get this completely wrong?

Re: [Xen-devel] [PATCH v1 1/1] xen/arm: Map mmio-sram nodes as cached memory

[Edgar E. Iglesias]

On Mon, Dec 19, 2016 at 04:01:00PM -0800, Stefano Stabellini wrote:
> On Mon, 19 Dec 2016, Julien Grall wrote:
> > Hi Edgar,
> > 
> > On 16/12/2016 18:04, Edgar E. Iglesias wrote:
> > > On Fri, Dec 16, 2016 at 04:12:00PM +, Julien Grall wrote:
> > > > On 15/12/16 11:26, Edgar E. Iglesias wrote:
> > > > > From: "Edgar E. Iglesias" 
> > > > > 
> > > > > Relax the mapping of mmio-sram nodes that do not set the
> > > > > no-memory-wc property to cached normal memory.
> > > > > 
> > > > > Rationale:
> > > > > Allthough on chip memories are relatively fast compared to
> > > > 
> > > > s/Allthough/Although/
> > > 
> > > Fixed for v2.
> > > 
> > > > 
> > > > > off-chip memories, large OCMs are still significantly slower
> > > > 
> > > > May I ask what does OCMs mean?
> > > 
> > > It means On Chip Memories. I can spell it out in v2.
> > 
> > Yes please. I was not able to find the acronym with a quick search Google.
> > 
> > [...]
> > 
> > > > I consider it as the most trusted domain and has other way to mess up 
> > > > the
> > > > platform. So I am fine with this relaxation for the hardware domain (AKA
> > > > DOM0).
> > > > 
> > > > However, I have the feeling that we need this kind of relaxation for 
> > > > many
> > > > devices. For instance prefetchable memory BARs for PCI would have to be
> > > > cacheable, same for integrated graphic cards.
> > > 
> > > Yes, I agree.
> > > 
> > > > 
> > > > I am wondering whether for DOM0 only (*not the other guests), we should
> > > > relax the default stage-2 attribute mapping to p2m_mmio_direct_c.
> > > > 
> > > > So we would let DOM0 in charge of the final attribute. This may boost 
> > > > the
> > > > performance of memory access in DOM0.
> > > > 
> > > > Any opinions?
> > > 
> > > I think it makes sense. We had a discussiong about it a while back ago:
> > > https://lists.xenproject.org/archives/html/xen-devel/2015-05/msg02349.html
> > > 
> > > The concerns that were raised were wether there could be devices that
> > > could behave badly and possibly giving dom0 access to trig undefined or
> > > unpredictable behavior that could be exploited.
> > 
> > I think it would be fine for DOM0. It is already a trusted domain and have
> > other way to take down the platform.
> > 
> > > 
> > > If dom0 accesses devices through a cache, access patterns will change.
> > > In theory it may trig unexpected behaviour in some device. It's hard
> > > to say unless talking about specific chips and implementations.
> > > 
> > > For example, given dom0 access to a DMA capable device may also achieve
> > > the same. I.e access patterns from DMA units differ from the ones
> > > originating from a CPU using uncached device memory. It could trig
> > > similar kind of errors.
> > 
> > Another example is having the device mapped with different in 2 places with
> > different cacheability attributes. The data accessed would not be coherent.
> > But I believe that should not lead to a security issue.
> > 
> > > 
> > > It would be interesting to see a concrete example of a problematic
> > > system.
> > 
> > I believe it would depend a lot on how the platform have been designed.
> > 
> > I think we have two choices to go forward:
> > 1) Relax the memory attribute on case by case. DOM0 would not be able
> > to exploit potential undefined behavior. However, this means that code is 
> > been
> > added for any new device (e.g compatible string).
> > 2) Relax the memory attribute on every case. DOM0 would be able to
> > exploit potential undefined behavior. On the benefit side, every devices can
> > be used at its full performance without any change required in Xen.
> > 
> > In the case of ACPI, we rely on DOM0 to provide the correct mapping 
> > attribute
> > when asking to do the stage-2 mapping (see XENMAPSPACE_dev_mmio). Note that
> > currently, the MMIO are always mapped with the attribute Device-nGnRE. There
> > is plan to change that in the future. So ACPI is implementing 2).
> > 
> > Device Tree is currently implementing 1). But I would like to see the 
> > behavior
> > of Xen the same no matter the firmware tables used. So I would lean towards
> > 2).
> 
> That's fine by me.


OK, I've sent a v2 of the mmio-sram patch to avoid delays with that part.

But now that we seem to be in agreement with going for relaxing the default
S2 mappings, perhaps we should ditch the whitelist/blacklist mechanism?
Or does someone see a need for a blacklist?

Cheers,
Edgar

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86/time: Adjust init-time handling of pit0_ticks

[Jan Beulich]

>>> On 20.12.16 at 13:17,  wrote:
> On 20/12/2016 07:25, Jan Beulich wrote:
> On 19.12.16 at 17:58,  wrote:
>>> On 19/12/16 16:51, Jan Beulich wrote:
>>> On 19.12.16 at 17:38,  wrote:
> There is no need for the volatile cast in the timer interrupt.  
> pit0_ticks has
> external linkage, preventing the compiler from eliding the update.  This
> reduces the generated assembly from a read, local modify, write to a 
> single
> add instruction.
 I don't think external linkage is the reason here, considering the
 effects of whole-program-optimization.
>>> In the case of whole-program-optimisation, the compiler would observe
>>> that one function wrote to the variable, and one function read from it. 
>>> I presume that is also sufficient to prevent the eliding?
>> I would think so, yes (albeit the end result of that process may
>> be that everything which isn't recursive and doesn't serve as
>> independent entry point ends up as a few huge functions); I
>> merely wanted to point out that linkage isn't really relevant here.
> 
> What about this?
> 
> There is no need for the volatile cast in the timer interrupt; the compiler
> may not elide the update.  This reduces the generated assembly from a read,
> local modify, write to a single add instruction.

Sounds fine.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen/x86: Improve hypercall page writing

[Jan Beulich]

>>> On 20.12.16 at 13:21,  wrote:
> On 20/12/2016 07:48, Jan Beulich wrote:
>>
>>> @@ -398,10 +400,11 @@ static void 
>>> hypercall_page_initialise_ring1_kernel(void 
> *hypercall_page)
>>>   * calling it.
>>>   */
>>>  p = (char *)(hypercall_page + (__HYPERVISOR_iret * 32));
>>> -*(u8  *)(p+ 0) = 0x50;/* push %eax */
>>> -*(u8  *)(p+ 1) = 0xb8;/* mov  $__HYPERVISOR_iret,%eax */
>>> -*(u32 *)(p+ 2) = __HYPERVISOR_iret;
>>> -*(u16 *)(p+ 6) = (HYPERCALL_VECTOR << 8) | 0xcd; /* int  $xx */
>>> +memcpy(p,
>>> +   "\x50" /* push %eax */
>>> +   "\xb8\x17\x00\x00\x00" /* mov  $__HYPERVISOR_iret, %eax */
>>> +   "\xcd\x82",/* int  $HYPERCALL_VECTOR */
>> here as a good idea.
> 
> Well, they are fixed in the ABI.  It is not as if we could ever change them.
> 
>>  If you used a static const uint8_t[] instead of
>> a string literal (which even includes a pointless nul terminator), all of
>> this could be avoided afaict.
> 
> I can see how that would work for the `int $0x82` case, but how do you
> propose fitting 4 bytes of __HYPERVISOR_iret into an initialiser for a
> uint8_t array?

One byte __HYPERVISOR_iret, the other three zeros. perhaps
accompanied by a BUILD_BUG_ON(). Another alternative would
be a static const struct.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 2/2] x86/emul: Bugfixes to SYSCALL emulation

[Jan Beulich]

>>> On 20.12.16 at 13:14,  wrote:
> On 20/12/2016 08:22, Jan Beulich wrote:
> On 19.12.16 at 17:37,  wrote:
>> What I've been thinking the other day though is: Why
>> don't we put the whole SYSCALL emulation into a __XEN__
>> conditional (implying __x86_64__, i.e. allowing the inner ones
>> to be removed)?
> 
> This depends on whether we think it is ever realistic to be able to be
> able to test things like this in the userspace harness.  I am not sure
> we realistically can.

Well - it's all about register modifications, which we surely could
test. But this would be a rather contrived test, the usefulness of
which I would question.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] merlot0 and merlot1 (was Re: [xen-4.4-testing test] 103756: trouble: blocked/broken/fail/pass)

[Jan Beulich]

>>> On 20.12.16 at 12:59,  wrote:
> osstest service owner writes ("[xen-4.4-testing test] 103756: trouble: 
> blocked/broken/fail/pass"):
>> flight 103756 xen-4.4-testing real [real]
>> http://logs.test-lab.xenproject.org/osstest/logs/103756/ 
>> 
>> Failures and problems with tests :-(
>> 
>> Tests which did not succeed and are blocking,
>> including tests which could not be run:
>>  build-i386-xend3 host-install(3)broken REGR. vs. 103438
> 
> merlot1 had lost its BIOS settings including boot order.  While
> repairing this I found that merlot0 had too.  I wonder if that might
> be a software-induced problem.

I consider this difficult to imagine, but it's also not entirely
impossible. Since this appears to recur, are there perhaps logs
(ideally more than one instance, to allow finding a possible
pattern) from right before the issue occurred?

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] x86/apicv: fix RTC periodic timer and apicv issue

[Xuquan (Quan Xu)]

On December 20, 2016 1:37 PM, Tian, Kevin wrote:
>> From: Xuquan (Quan Xu) [mailto:xuqu...@huawei.com]
>> Sent: Friday, December 16, 2016 5:40 PM
>I suppose you've verified this new version, but still would like get your
>explicit confirmation - did you still see time accuracy issue in your side?
>Have you tried other guest OS types other than Win7-32?
>

Kevin, I have tested it again..

__without__ my patch, for win7-64, the wall clock time looks working fine..
It seems the issue is only for win-32..

There is a easy way to reproduce:
*pCPU should be v3 ..(my pCPU is """Intel(R) Xeon(R) CPU E5-2620 v3 @ 
2.40GHz""")
* more than 2 vCPUs for win32 guest.

Than run the following .bat in win32 guest:

:abcd
echo 1
goto :abcd





Could Intel test team help me verify it?

>> ---
>>  xen/arch/x86/hvm/vmx/intr.c | 15 ---
>>  1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/arch/x86/hvm/vmx/intr.c b/xen/arch/x86/hvm/vmx/intr.c
>> index 639a705..d7a5716 100644
>> --- a/xen/arch/x86/hvm/vmx/intr.c
>> +++ b/xen/arch/x86/hvm/vmx/intr.c
>> -if (pt_vector != -1)
>> -vmx_set_eoi_exit_bitmap(v, pt_vector);
>> +if ( pt_vector != -1 ) {
>> +if ( intack.vector > pt_vector )
>> +vmx_set_eoi_exit_bitmap(v, intack.vector);
>> +else
>> +vmx_set_eoi_exit_bitmap(v, pt_vector);
>> +}
>
>Above can be simplified as one line change:
>   if ( pt_vector != -1 )
>   vmx_set_eoi_exit_bitmap(v, intack.vector);
>

I have verified this change.. it is working..
Could I send out v4 with this changes?


Quan

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] xen: sched: removal of redundant check in Credit

[Dario Faggioli]

On Tue, 2016-12-20 at 14:34 +0530, Praveen Kumar wrote:
> Hi Dario,
> 
> I tried with 'git am' to apply the patch after downloading the mbox
> file, that worked fine. Do let me know if that is ok.
> 
Well, if you tried and it works, I'm sure it is. I don't really use
`git am` as part of my workload, that's why I did not try.

I've now done it as a test, and I confirm it works. So it's probably an
StGit issue, or something else. In fact, I said in my email that,
although I was reporting an issue I was having, I wasn't sure how
relevant it was, and could not rule out it was an issue with my own
setup.

Thanks for trying `git am` and reporting it works. This patch has my
Ack already, which means I'm happy with it. It's now up to George, or
another committer, to either comment or put it in.

Let's a little bit of time for this to happen, especially considering
the time of the year. :-D
If it will take too long, you can ping the thread with an email.

Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 06/13] x86/domctl: Handle ACPI access from domctl

[Jan Beulich]

>>> On 17.12.16 at 00:18,  wrote:
> @@ -32,14 +34,15 @@ static int acpi_cpumap_access_common(struct domain *d,
>  memcpy(val, (uint8_t *)d->avail_vcpus + first_byte,
> min(bytes, ((d->max_vcpus + 7) / 8) - first_byte));
>  }
> -else
> +else if ( !is_guest_access )
>  /* Guests do not write CPU map */
> -return X86EMUL_UNHANDLEABLE;
> +memcpy((uint8_t *)d->avail_vcpus + first_byte, val,
> +   min(bytes, ((d->max_vcpus + 7) / 8) - first_byte));
>  
>  return X86EMUL_OKAY;
>  }

So you're changing to return OKAY even in the guest-write case -
I don't think that's what you want.

> -static int acpi_access_common(struct domain *d,
> +static int acpi_access_common(struct domain *d, bool is_guest_access,

Why? I thought the domctl is needed only for updating the CPU
map? Or maybe it would help if the patch had a non-empty
description.

> @@ -129,13 +138,50 @@ int hvm_acpi_domctl_access(struct domain *d, uint8_t rw,
> const xen_acpi_access_t *access,
> XEN_GUEST_HANDLE_PARAM(void) arg)
>  {
> -return -ENOSYS;
> +unsigned int bytes, i;
> +uint32_t val;
> +uint8_t *ptr = (uint8_t *)&val;
> +int rc;
> +int (*do_acpi_access)(struct domain *d, bool is_guest_access,
> +  int dir, unsigned int port,
> +  unsigned int bytes, uint32_t *val);
> +
> +if ( has_acpi_dm_ff(d) )
> +return -EINVAL;

Perhaps better EOPNOTSUPP or ENODEV?

> +if ( access->space_id != XEN_ACPI_SYSTEM_IO )
> +return -EINVAL;
> +
> +if ( (access->address >= XEN_ACPI_CPU_MAP) &&
> + (access->address < XEN_ACPI_CPU_MAP + XEN_ACPI_CPU_MAP_LEN) )
> +do_acpi_access = acpi_cpumap_access_common;
> +else
> +do_acpi_access = acpi_access_common;
> +
> +for ( i = 0; i < access->width; i += sizeof(val) )
> +{
> +bytes = (access->width - i > sizeof(val)) ? sizeof(val) : 
> access->width - i;
> +
> +if ( (rw == XEN_DOMCTL_ACPI_WRITE) &&
> + copy_from_guest_offset(ptr, arg, i, bytes) )
> +return -EFAULT;
> +
> +rc = do_acpi_access(d, false, rw, access->address, bytes, &val);
> +if ( rc )
> +return rc;
> +
> +if ( (rw == XEN_DOMCTL_ACPI_READ) &&
> + copy_to_guest_offset(arg, i, ptr, bytes) )
> +return -EFAULT;
> +}

I could imagine Coverity considering val potentially uninitialized
with this logic, i.e. I think we'd be better off if it had an
initializer.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 08/13] pvh: Send an SCI on VCPU hotplug event

[Jan Beulich]

>>> On 17.12.16 at 00:18,  wrote:
> @@ -128,6 +130,13 @@ static int acpi_access_common(struct domain *d, bool 
> is_guest_access,
>  *en = (((v & 0xff) << 8) | (*en & 0xff)) & *mask_en;
>  break;
>  }
> +
> +/*
> + * If a new bit has been set in status register and corresponding
> + * event is enabled then an SCI is sent to the guest.
> + */
> +if ( !is_guest_access && ((*sts & ~sts_orig) & *en) )
> +send_guest_global_virq(d, VIRQ_SCI);

From an abstract pov: How a bit gets set in either register shouldn't
matter. Hence the !is_guest_access part here is at least questionable.
However, iirc the guest can't itself set any status bits (since they're
write-1-clear). Otoh, when an enable bit transitions from 0 to 1 (which
the guest can effect) and the respective status bit is set, an SCI
should occur according to my reading of the spec.

Also - indentation.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 12/13] pvh/acpi: Save ACPI registers for PVH guests

[Jan Beulich]

>>> On 17.12.16 at 00:18,  wrote:
> --- a/xen/arch/x86/hvm/pmtimer.c
> +++ b/xen/arch/x86/hvm/pmtimer.c
> @@ -257,7 +257,11 @@ static int acpi_save(struct domain *d, 
> hvm_domain_context_t *h)
>  int rc;
>  
>  if ( !has_vpm(d) )
> +{
> +if ( !has_acpi_dm_ff(d) )
> +return hvm_save_entry(PMTIMER, 0, h, acpi);
>  return 0;
> +}
>  
>  spin_lock(&s->lock);
>  
> @@ -286,7 +290,11 @@ static int acpi_load(struct domain *d, 
> hvm_domain_context_t *h)
>  PMTState *s = &d->arch.hvm_domain.pl_time->vpmt;
>  
>  if ( !has_vpm(d) )
> +{
> +if ( !has_acpi_dm_ff(d) )
> +return hvm_load_entry(PMTIMER, h, acpi);
>  return -ENODEV;
> +}
>  
>  spin_lock(&s->lock);

Seeing this I first of all wonder - would there be any harm in simply
having PVH take (almost) the same route as HVM here? In particular
there's a pmt_update_sci() call, an equivalent of which would seem
to be needed for PVH too.

Which in turn gets me to wonder whether some of the code which
is already there couldn't be re-used (handle_evt_io() for example).

And then, seeing the locking here - don't you need some locking
in the earlier patches too, both to serialize accesses from multiple
guest vCPU-s and to arbitrate between Dom0 and the guest?

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH] xen/blkback: use rb_entry()

[Geliang Tang]

To make the code clearer, use rb_entry() instead of container_of() to
deal with rbtree.

Signed-off-by: Geliang Tang 
---
 drivers/block/xen-blkback/blkback.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/block/xen-blkback/blkback.c 
b/drivers/block/xen-blkback/blkback.c
index 726c32e..7e59fae 100644
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -191,10 +191,10 @@ static void make_response(struct xen_blkif_ring *ring, 
u64 id,
  unsigned short op, int st);
 
 #define foreach_grant_safe(pos, n, rbtree, node) \
-   for ((pos) = container_of(rb_first((rbtree)), typeof(*(pos)), node), \
+   for ((pos) = rb_entry(rb_first((rbtree)), typeof(*(pos)), node), \
 (n) = (&(pos)->node != NULL) ? rb_next(&(pos)->node) : NULL; \
 &(pos)->node != NULL; \
-(pos) = container_of(n, typeof(*(pos)), node), \
+(pos) = rb_entry(n, typeof(*(pos)), node), \
 (n) = (&(pos)->node != NULL) ? rb_next(&(pos)->node) : NULL)
 
 
@@ -223,7 +223,7 @@ static int add_persistent_gnt(struct xen_blkif_ring *ring,
/* Figure out where to put new node */
new = &ring->persistent_gnts.rb_node;
while (*new) {
-   this = container_of(*new, struct persistent_gnt, node);
+   this = rb_entry(*new, struct persistent_gnt, node);
 
parent = *new;
if (persistent_gnt->gnt < this->gnt)
@@ -254,7 +254,7 @@ static struct persistent_gnt *get_persistent_gnt(struct 
xen_blkif_ring *ring,
 
node = ring->persistent_gnts.rb_node;
while (node) {
-   data = container_of(node, struct persistent_gnt, node);
+   data = rb_entry(node, struct persistent_gnt, node);
 
if (gref < data->gnt)
node = node->rb_left;
-- 
2.9.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH] xen/evtchn: use rb_entry()

[Geliang Tang]

To make the code clearer, use rb_entry() instead of container_of() to
deal with rbtree.

Signed-off-by: Geliang Tang 
---
 drivers/xen/evtchn.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/evtchn.c b/drivers/xen/evtchn.c
index e8c7f09..6890897 100644
--- a/drivers/xen/evtchn.c
+++ b/drivers/xen/evtchn.c
@@ -125,7 +125,7 @@ static int add_evtchn(struct per_user_data *u, struct 
user_evtchn *evtchn)
while (*new) {
struct user_evtchn *this;
 
-   this = container_of(*new, struct user_evtchn, node);
+   this = rb_entry(*new, struct user_evtchn, node);
 
parent = *new;
if (this->port < evtchn->port)
@@ -157,7 +157,7 @@ static struct user_evtchn *find_evtchn(struct per_user_data 
*u, unsigned port)
while (node) {
struct user_evtchn *evtchn;
 
-   evtchn = container_of(node, struct user_evtchn, node);
+   evtchn = rb_entry(node, struct user_evtchn, node);
 
if (evtchn->port < port)
node = node->rb_left;
-- 
2.9.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 04/13] pvh/acpi: Install handlers for ACPI-related PVH IO accesses

[Boris Ostrovsky]

On 12/20/2016 06:24 AM, Jan Beulich wrote:
 On 17.12.16 at 00:18,  wrote:
>> --- a/xen/include/public/arch-x86/hvm/save.h
>> +++ b/xen/include/public/arch-x86/hvm/save.h
>> @@ -527,7 +527,37 @@ DECLARE_HVM_SAVE_TYPE(HPET, 12, struct hvm_hw_hpet);
>>  /*
>>   * PM timer
>>   */
>> +#if __XEN_INTERFACE_VERSION__ >= 0x00040800
>> +struct hvm_hw_pmtimer {
>> +uint32_t tmr_val;   /* PM_TMR_BLK.TMR_VAL: 32bit free-running counter */
>> +uint16_t pm1a_sts;  /* PM1a_EVT_BLK.PM1a_STS: status register */
>> +uint16_t pm1a_en;   /* PM1a_EVT_BLK.PM1a_EN: enable register */
>> +#if defined(__XEN__) || defined(__XEN_TOOLS__)
>> +uint16_t gpe0_sts;
>> +uint16_t gpe0_en;
>> +#endif
> Why inside another #ifdef? There's no other example in this file
> which might have suggested to you that it needs doing this way.
> In fact there are also no pre-existing uses of
> __XEN_INTERFACE_VERSION__ in this header, so I also don't see
> why you added one (and then with a slightly off value check).

Don't we want users of old interface to continue using original
definition of hvm_hw_timer? And not to expose them to the fix routine below?

-boris

>
> If anything the _whole_ header would need to become Xen/tools
> only.
>
>> +static inline int _hvm_hw_fix_pmtimer(void *h, uint32_t size)
>> +{
>> +#if defined(__XEN__) || defined(__XEN_TOOLS__)
>> +struct hvm_hw_pmtimer *acpi = (struct hvm_hw_pmtimer *)h;
>> +
>> +if ( size == sizeof(struct hvm_hw_pmtimer_compat) )
>> +acpi->gpe0_sts = acpi->gpe0_en = 0;
>> +#endif
> Same here.
>
> Jan
>



___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 04/13] pvh/acpi: Install handlers for ACPI-related PVH IO accesses

[Jan Beulich]

>>> On 20.12.16 at 15:03,  wrote:
> On 12/20/2016 06:24 AM, Jan Beulich wrote:
> On 17.12.16 at 00:18,  wrote:
>>> --- a/xen/include/public/arch-x86/hvm/save.h
>>> +++ b/xen/include/public/arch-x86/hvm/save.h
>>> @@ -527,7 +527,37 @@ DECLARE_HVM_SAVE_TYPE(HPET, 12, struct hvm_hw_hpet);
>>>  /*
>>>   * PM timer
>>>   */
>>> +#if __XEN_INTERFACE_VERSION__ >= 0x00040800
>>> +struct hvm_hw_pmtimer {
>>> +uint32_t tmr_val;   /* PM_TMR_BLK.TMR_VAL: 32bit free-running counter 
>>> */
>>> +uint16_t pm1a_sts;  /* PM1a_EVT_BLK.PM1a_STS: status register */
>>> +uint16_t pm1a_en;   /* PM1a_EVT_BLK.PM1a_EN: enable register */
>>> +#if defined(__XEN__) || defined(__XEN_TOOLS__)
>>> +uint16_t gpe0_sts;
>>> +uint16_t gpe0_en;
>>> +#endif
>> Why inside another #ifdef? There's no other example in this file
>> which might have suggested to you that it needs doing this way.
>> In fact there are also no pre-existing uses of
>> __XEN_INTERFACE_VERSION__ in this header, so I also don't see
>> why you added one (and then with a slightly off value check).
> 
> Don't we want users of old interface to continue using original
> definition of hvm_hw_timer? And not to expose them to the fix routine below?

There shouldn't be any such old users, because of ...

>> If anything the _whole_ header would need to become Xen/tools
>> only.

... this.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 04/13] pvh/acpi: Install handlers for ACPI-related PVH IO accesses

[Boris Ostrovsky]

On 12/20/2016 09:10 AM, Jan Beulich wrote:
 On 20.12.16 at 15:03,  wrote:
>> On 12/20/2016 06:24 AM, Jan Beulich wrote:
>> On 17.12.16 at 00:18,  wrote:
 --- a/xen/include/public/arch-x86/hvm/save.h
 +++ b/xen/include/public/arch-x86/hvm/save.h
 @@ -527,7 +527,37 @@ DECLARE_HVM_SAVE_TYPE(HPET, 12, struct hvm_hw_hpet);
  /*
   * PM timer
   */
 +#if __XEN_INTERFACE_VERSION__ >= 0x00040800
 +struct hvm_hw_pmtimer {
 +uint32_t tmr_val;   /* PM_TMR_BLK.TMR_VAL: 32bit free-running counter 
 */
 +uint16_t pm1a_sts;  /* PM1a_EVT_BLK.PM1a_STS: status register */
 +uint16_t pm1a_en;   /* PM1a_EVT_BLK.PM1a_EN: enable register */
 +#if defined(__XEN__) || defined(__XEN_TOOLS__)
 +uint16_t gpe0_sts;
 +uint16_t gpe0_en;
 +#endif
>>> Why inside another #ifdef? There's no other example in this file
>>> which might have suggested to you that it needs doing this way.
>>> In fact there are also no pre-existing uses of
>>> __XEN_INTERFACE_VERSION__ in this header, so I also don't see
>>> why you added one (and then with a slightly off value check).
>> Don't we want users of old interface to continue using original
>> definition of hvm_hw_timer? And not to expose them to the fix routine below?
> There shouldn't be any such old users, because of ...
>
>>> If anything the _whole_ header would need to become Xen/tools
>>> only.
> ... this.


Is this file is not supposed to be used by anyone outside of the Xen tree?


-boris

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 05/13] pvh/acpi: Handle ACPI accesses for PVH guests

[Boris Ostrovsky]

On 12/20/2016 06:50 AM, Jan Beulich wrote:
>
>> +
>> +if ( dir == XEN_DOMCTL_ACPI_READ )
>> +{
>> +uint32_t mask = (bytes < 4) ? ~0U << (bytes * 8) : 0;
>> +uint32_t data = (((uint32_t)*en) << 16) | *sts;
> There's one pair of pointless parentheses around the cast
> expression here.
>
>> +
>> +data >>= 8 * (port & 3);
>> +*val = (*val & mask) | (data & ~mask);
>> +}
>> +else
>> +{
>> +uint32_t v = *val;
>> +
>> +/* Status register is write-1-to-clear by guests */
>> +switch ( port & 3 )
>> +{
>> +case 0:
>> +*sts &= ~(v & 0xff);
>> +*sts &= *mask_sts;
> I can understand the first &=, but why would a read have this second
> (side) effect? I could see some sort of need for such only when you
> were setting any flags.

This is a write, not a read.

>
>> @@ -18,13 +135,19 @@ int hvm_acpi_domctl_access(struct domain *d, uint8_t rw,
>>  static int acpi_guest_access(int dir, unsigned int port,
>>   unsigned int bytes, uint32_t *val)
>>  {
>> -return X86EMUL_UNHANDLEABLE;
>> +return  acpi_access_common(current->domain,
>> +   (dir == IOREQ_READ) ?
>> +   XEN_DOMCTL_ACPI_READ: XEN_DOMCTL_ACPI_WRITE,
>> +   port, bytes, val);
>>  }
> I don't think this one is meant to be used by the domctl, so I don't see
> why you need a helper here. That would also eliminate the seemingly
> unnecessary use of XEN_DOMCTL_* here (I think you already had said
> this was an oversight in an earlier reply).

Yes, XEN_DOMCTL_*is out of place here but I'd prefer to keep the helper
to isolate the sense of 'dir' as is used by portio handling
infrastructure (i.e. IOREQ_READ/WRITE) from how it is defined in the
acpi code here. Especially if, as mentioned in another thread, I use
bool is_write in common routines.


-boris


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 06/13] x86/domctl: Handle ACPI access from domctl

[Boris Ostrovsky]

On 12/20/2016 08:24 AM, Jan Beulich wrote:
>
>> -static int acpi_access_common(struct domain *d,
>> +static int acpi_access_common(struct domain *d, bool is_guest_access,
> Why? I thought the domctl is needed only for updating the CPU
> map? Or maybe it would help if the patch had a non-empty
> description.

domctl updates both the map and the status. I.e. in the toolstack it
looks like

/*Update VCPU map. */
rc = xc_acpi_iowrite(CTX->xch, domid, XEN_ACPI_CPU_MAP,
 cpumap->size, cpumap->map);
if (!rc) {
/* Send an SCI. */
uint16_t val = 1 << XEN_ACPI_GPE0_CPUHP_BIT;
rc = xc_acpi_iowrite(CTX->xch, domid, ACPI_GPE0_BLK_ADDRESS_V1,
 sizeof(val), &val);
}


I'll make a note in the commit message of the fact that both are accessed.

OTOH, maybe we should have an update to the map trigger the SCI and not
require the toolstack to do so?

-boris


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 04/13] pvh/acpi: Install handlers for ACPI-related PVH IO accesses

[Jan Beulich]

>>> On 20.12.16 at 15:16,  wrote:
> On 12/20/2016 09:10 AM, Jan Beulich wrote:
> On 20.12.16 at 15:03,  wrote:
>>> On 12/20/2016 06:24 AM, Jan Beulich wrote:
>>> On 17.12.16 at 00:18,  wrote:
> --- a/xen/include/public/arch-x86/hvm/save.h
> +++ b/xen/include/public/arch-x86/hvm/save.h
> @@ -527,7 +527,37 @@ DECLARE_HVM_SAVE_TYPE(HPET, 12, struct hvm_hw_hpet);
>  /*
>   * PM timer
>   */
> +#if __XEN_INTERFACE_VERSION__ >= 0x00040800
> +struct hvm_hw_pmtimer {
> +uint32_t tmr_val;   /* PM_TMR_BLK.TMR_VAL: 32bit free-running 
> counter */
> +uint16_t pm1a_sts;  /* PM1a_EVT_BLK.PM1a_STS: status register */
> +uint16_t pm1a_en;   /* PM1a_EVT_BLK.PM1a_EN: enable register */
> +#if defined(__XEN__) || defined(__XEN_TOOLS__)
> +uint16_t gpe0_sts;
> +uint16_t gpe0_en;
> +#endif
 Why inside another #ifdef? There's no other example in this file
 which might have suggested to you that it needs doing this way.
 In fact there are also no pre-existing uses of
 __XEN_INTERFACE_VERSION__ in this header, so I also don't see
 why you added one (and then with a slightly off value check).
>>> Don't we want users of old interface to continue using original
>>> definition of hvm_hw_timer? And not to expose them to the fix routine below?
>> There shouldn't be any such old users, because of ...
>>
 If anything the _whole_ header would need to become Xen/tools
 only.
>> ... this.
> 
> Is this file is not supposed to be used by anyone outside of the Xen tree?

I don't think so, no. In any event - prior additions did not do
any precautions to guard possible foreign consumers. Maybe
Andrew has an opinion here ...

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 05/13] pvh/acpi: Handle ACPI accesses for PVH guests

[Jan Beulich]

>>> On 20.12.16 at 15:35,  wrote:
> On 12/20/2016 06:50 AM, Jan Beulich wrote:
>>> +else
>>> +{
>>> +uint32_t v = *val;
>>> +
>>> +/* Status register is write-1-to-clear by guests */
>>> +switch ( port & 3 )
>>> +{
>>> +case 0:
>>> +*sts &= ~(v & 0xff);
>>> +*sts &= *mask_sts;
>> I can understand the first &=, but why would a read have this second
>> (side) effect? I could see some sort of need for such only when you
>> were setting any flags.
> 
> This is a write, not a read.

Oh, right. But the question remains about that unexpected side
effect.

>>> @@ -18,13 +135,19 @@ int hvm_acpi_domctl_access(struct domain *d, uint8_t 
>>> rw,
>>>  static int acpi_guest_access(int dir, unsigned int port,
>>>   unsigned int bytes, uint32_t *val)
>>>  {
>>> -return X86EMUL_UNHANDLEABLE;
>>> +return  acpi_access_common(current->domain,
>>> +   (dir == IOREQ_READ) ?
>>> +   XEN_DOMCTL_ACPI_READ: XEN_DOMCTL_ACPI_WRITE,
>>> +   port, bytes, val);
>>>  }
>> I don't think this one is meant to be used by the domctl, so I don't see
>> why you need a helper here. That would also eliminate the seemingly
>> unnecessary use of XEN_DOMCTL_* here (I think you already had said
>> this was an oversight in an earlier reply).
> 
> Yes, XEN_DOMCTL_*is out of place here but I'd prefer to keep the helper
> to isolate the sense of 'dir' as is used by portio handling
> infrastructure (i.e. IOREQ_READ/WRITE) from how it is defined in the
> acpi code here. Especially if, as mentioned in another thread, I use
> bool is_write in common routines.

Well, if the helper is needed for the domctl, then this is at least
acceptable (albeit personally I'd prefer the domctl handler to
do the translation, using the IOREQ_* values internally. If the
helper is not needed for domctl, then XEN_DOMCTL_* shouldn't
be passed here.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 06/13] x86/domctl: Handle ACPI access from domctl

[Jan Beulich]

>>> On 20.12.16 at 15:45,  wrote:
> On 12/20/2016 08:24 AM, Jan Beulich wrote:
>>
>>> -static int acpi_access_common(struct domain *d,
>>> +static int acpi_access_common(struct domain *d, bool is_guest_access,
>> Why? I thought the domctl is needed only for updating the CPU
>> map? Or maybe it would help if the patch had a non-empty
>> description.
> 
> domctl updates both the map and the status. I.e. in the toolstack it
> looks like
> 
> /*Update VCPU map. */
> rc = xc_acpi_iowrite(CTX->xch, domid, XEN_ACPI_CPU_MAP,
>  cpumap->size, cpumap->map);
> if (!rc) {
> /* Send an SCI. */
> uint16_t val = 1 << XEN_ACPI_GPE0_CPUHP_BIT;
> rc = xc_acpi_iowrite(CTX->xch, domid, ACPI_GPE0_BLK_ADDRESS_V1,
>  sizeof(val), &val);
> }
> 
> 
> I'll make a note in the commit message of the fact that both are accessed.
> 
> OTOH, maybe we should have an update to the map trigger the SCI and not
> require the toolstack to do so?

That would make sense, I think.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 04/13] pvh/acpi: Install handlers for ACPI-related PVH IO accesses

[Andrew Cooper]

On 20/12/2016 14:45, Jan Beulich wrote:
 On 20.12.16 at 15:16,  wrote:
>> On 12/20/2016 09:10 AM, Jan Beulich wrote:
>> On 20.12.16 at 15:03,  wrote:
 On 12/20/2016 06:24 AM, Jan Beulich wrote:
 On 17.12.16 at 00:18,  wrote:
>> --- a/xen/include/public/arch-x86/hvm/save.h
>> +++ b/xen/include/public/arch-x86/hvm/save.h
>> @@ -527,7 +527,37 @@ DECLARE_HVM_SAVE_TYPE(HPET, 12, struct hvm_hw_hpet);
>>  /*
>>   * PM timer
>>   */
>> +#if __XEN_INTERFACE_VERSION__ >= 0x00040800
>> +struct hvm_hw_pmtimer {
>> +uint32_t tmr_val;   /* PM_TMR_BLK.TMR_VAL: 32bit free-running 
>> counter */
>> +uint16_t pm1a_sts;  /* PM1a_EVT_BLK.PM1a_STS: status register */
>> +uint16_t pm1a_en;   /* PM1a_EVT_BLK.PM1a_EN: enable register */
>> +#if defined(__XEN__) || defined(__XEN_TOOLS__)
>> +uint16_t gpe0_sts;
>> +uint16_t gpe0_en;
>> +#endif
> Why inside another #ifdef? There's no other example in this file
> which might have suggested to you that it needs doing this way.
> In fact there are also no pre-existing uses of
> __XEN_INTERFACE_VERSION__ in this header, so I also don't see
> why you added one (and then with a slightly off value check).
 Don't we want users of old interface to continue using original
 definition of hvm_hw_timer? And not to expose them to the fix routine 
 below?
>>> There shouldn't be any such old users, because of ...
>>>
> If anything the _whole_ header would need to become Xen/tools
> only.
>>> ... this.
>> Is this file is not supposed to be used by anyone outside of the Xen tree?
> I don't think so, no. In any event - prior additions did not do
> any precautions to guard possible foreign consumers. Maybe
> Andrew has an opinion here ...

Our policies along these lines are a mess.  We really should separate
the guest ABI/API from the toolstack ABI/API, and by this, I mean having
separate directory structures.

In the meantime, all of the content in this file is only useful to
entities which can make DOMCTLs to start with, so the entire file should
be restricted to Xen/tools only.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 08/13] pvh: Send an SCI on VCPU hotplug event

[Boris Ostrovsky]

On 12/20/2016 08:37 AM, Jan Beulich wrote:
 On 17.12.16 at 00:18,  wrote:
>> @@ -128,6 +130,13 @@ static int acpi_access_common(struct domain *d, bool 
>> is_guest_access,
>>  *en = (((v & 0xff) << 8) | (*en & 0xff)) & *mask_en;
>>  break;
>>  }
>> +
>> +/*
>> + * If a new bit has been set in status register and corresponding
>> + * event is enabled then an SCI is sent to the guest.
>> + */
>> +if ( !is_guest_access && ((*sts & ~sts_orig) & *en) )
>> +send_guest_global_virq(d, VIRQ_SCI);
> From an abstract pov: How a bit gets set in either register shouldn't
> matter. Hence the !is_guest_access part here is at least questionable.
> However, iirc the guest can't itself set any status bits (since they're
> write-1-clear). Otoh, when an enable bit transitions from 0 to 1 (which
> the guest can effect) and the respective status bit is set, an SCI
> should occur according to my reading of the spec.

I haven't considered the case of enabling with a pending status. Yes,
that should trigger an SCI too, irrespective of is_guest_access.

And I guess since guests can't set status bits I can drop the
is_guest_access test in the check above too.

-boris


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 12/13] pvh/acpi: Save ACPI registers for PVH guests

[Boris Ostrovsky]

On 12/20/2016 08:57 AM, Jan Beulich wrote:
 On 17.12.16 at 00:18,  wrote:
>> --- a/xen/arch/x86/hvm/pmtimer.c
>> +++ b/xen/arch/x86/hvm/pmtimer.c
>> @@ -257,7 +257,11 @@ static int acpi_save(struct domain *d, 
>> hvm_domain_context_t *h)
>>  int rc;
>>  
>>  if ( !has_vpm(d) )
>> +{
>> +if ( !has_acpi_dm_ff(d) )
>> +return hvm_save_entry(PMTIMER, 0, h, acpi);
>>  return 0;
>> +}
>>  
>>  spin_lock(&s->lock);
>>  
>> @@ -286,7 +290,11 @@ static int acpi_load(struct domain *d, 
>> hvm_domain_context_t *h)
>>  PMTState *s = &d->arch.hvm_domain.pl_time->vpmt;
>>  
>>  if ( !has_vpm(d) )
>> +{
>> +if ( !has_acpi_dm_ff(d) )
>> +return hvm_load_entry(PMTIMER, h, acpi);
>>  return -ENODEV;
>> +}
>>  
>>  spin_lock(&s->lock);
> Seeing this I first of all wonder - would there be any harm in simply
> having PVH take (almost) the same route as HVM here? In particular
> there's a pmt_update_sci() call, an equivalent of which would seem
> to be needed for PVH too.

It probably is harmless but not very useful too since we'd be saving
PMTState which is not in use by PVH.

As far as pmt_update_sci() --- the only case when we have an SCI
generated is CPU map update and the interrupt is made pending to the
guest immediately so if we try to resend it during restore won't we be
sending it twice?

>
> Which in turn gets me to wonder whether some of the code which
> is already there couldn't be re-used (handle_evt_io() for example).

My plan is to merge these routines (to some extent) when we make HVM
guests use the same CPU hotplug code as PVH (i.e. stop using QEMU for that).


> And then, seeing the locking here - don't you need some locking
> in the earlier patches too, both to serialize accesses from multiple
> guest vCPU-s and to arbitrate between Dom0 and the guest?

Yes, that was missing.

-boris


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 05/13] pvh/acpi: Handle ACPI accesses for PVH guests

[Boris Ostrovsky]

On 12/20/2016 09:47 AM, Jan Beulich wrote:
 On 20.12.16 at 15:35,  wrote:
>> On 12/20/2016 06:50 AM, Jan Beulich wrote:
 +else
 +{
 +uint32_t v = *val;
 +
 +/* Status register is write-1-to-clear by guests */
 +switch ( port & 3 )
 +{
 +case 0:
 +*sts &= ~(v & 0xff);
 +*sts &= *mask_sts;
>>> I can understand the first &=, but why would a read have this second
>>> (side) effect? I could see some sort of need for such only when you
>>> were setting any flags.
>> This is a write, not a read.
> Oh, right. But the question remains about that unexpected side
> effect.

It indeed doesn't do anything for the case of guest access. It is
guarding against setting unauthorized bits by domctl (introduced by the
next patch). And I can move it into that case.

However, as discussed in the thread about 06/13 patch, we may currently
not need to provide domctl access to anything but VCPU map. So the
question is whether domctl interface to GPE0/PM1a is needed at all. I
think it's a useful interface with very little extra code required and
the toolstack can use it, for example, to inject events into guests. But
I don't have a specific use case.

-boris



___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 04/13] pvh/acpi: Install handlers for ACPI-related PVH IO accesses

[Boris Ostrovsky]

On 12/20/2016 09:55 AM, Andrew Cooper wrote:
>>> Is this file is not supposed to be used by anyone outside of the Xen tree?
>> I don't think so, no. In any event - prior additions did not do
>> any precautions to guard possible foreign consumers. Maybe
>> Andrew has an opinion here ...
> Our policies along these lines are a mess.  We really should separate
> the guest ABI/API from the toolstack ABI/API, and by this, I mean having
> separate directory structures.
>
> In the meantime, all of the content in this file is only useful to
> entities which can make DOMCTLs to start with, so the entire file should
> be restricted to Xen/tools only.

I can add that but it should be done as a separate patch as it has
nothing to do with what the series is trying to provide.

-boris

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 12/13] pvh/acpi: Save ACPI registers for PVH guests

[Jan Beulich]

>>> On 20.12.16 at 16:09,  wrote:
> On 12/20/2016 08:57 AM, Jan Beulich wrote:
> On 17.12.16 at 00:18,  wrote:
>>> --- a/xen/arch/x86/hvm/pmtimer.c
>>> +++ b/xen/arch/x86/hvm/pmtimer.c
>>> @@ -257,7 +257,11 @@ static int acpi_save(struct domain *d, 
>>> hvm_domain_context_t *h)
>>>  int rc;
>>>  
>>>  if ( !has_vpm(d) )
>>> +{
>>> +if ( !has_acpi_dm_ff(d) )
>>> +return hvm_save_entry(PMTIMER, 0, h, acpi);
>>>  return 0;
>>> +}
>>>  
>>>  spin_lock(&s->lock);
>>>  
>>> @@ -286,7 +290,11 @@ static int acpi_load(struct domain *d, 
>>> hvm_domain_context_t *h)
>>>  PMTState *s = &d->arch.hvm_domain.pl_time->vpmt;
>>>  
>>>  if ( !has_vpm(d) )
>>> +{
>>> +if ( !has_acpi_dm_ff(d) )
>>> +return hvm_load_entry(PMTIMER, h, acpi);
>>>  return -ENODEV;
>>> +}
>>>  
>>>  spin_lock(&s->lock);
>> Seeing this I first of all wonder - would there be any harm in simply
>> having PVH take (almost) the same route as HVM here? In particular
>> there's a pmt_update_sci() call, an equivalent of which would seem
>> to be needed for PVH too.
> 
> It probably is harmless but not very useful too since we'd be saving
> PMTState which is not in use by PVH.
> 
> As far as pmt_update_sci() --- the only case when we have an SCI
> generated is CPU map update and the interrupt is made pending to the
> guest immediately so if we try to resend it during restore won't we be
> sending it twice?

Well, depends on how far things got prior to the (racing) migration.
I think it's better to send one too many than risking the guest not
getting to see any.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 05/13] pvh/acpi: Handle ACPI accesses for PVH guests

[Jan Beulich]

>>> On 20.12.16 at 16:29,  wrote:
> On 12/20/2016 09:47 AM, Jan Beulich wrote:
> On 20.12.16 at 15:35,  wrote:
>>> On 12/20/2016 06:50 AM, Jan Beulich wrote:
> +else
> +{
> +uint32_t v = *val;
> +
> +/* Status register is write-1-to-clear by guests */
> +switch ( port & 3 )
> +{
> +case 0:
> +*sts &= ~(v & 0xff);
> +*sts &= *mask_sts;
 I can understand the first &=, but why would a read have this second
 (side) effect? I could see some sort of need for such only when you
 were setting any flags.
>>> This is a write, not a read.
>> Oh, right. But the question remains about that unexpected side
>> effect.
> 
> It indeed doesn't do anything for the case of guest access. It is
> guarding against setting unauthorized bits by domctl (introduced by the
> next patch). And I can move it into that case.
> 
> However, as discussed in the thread about 06/13 patch, we may currently
> not need to provide domctl access to anything but VCPU map. So the
> question is whether domctl interface to GPE0/PM1a is needed at all. I
> think it's a useful interface with very little extra code required and
> the toolstack can use it, for example, to inject events into guests. But
> I don't have a specific use case.

To be honest, I'd keep out anything we don't really need.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [xen-unstable-smoke test] 103774: regressions - FAIL

[osstest service owner]

flight 103774 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/103774/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-libvirt17 guest-start/debian.repeat fail REGR. vs. 103771

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  8867a48e058ab04bd269cab12ef4de5ee8ad5b7b
baseline version:
 xen  d6ac8e22c7c5525db1da79fd1d1f03ee6b557f0d

Last test of basis   103771  2016-12-20 09:02:14 Z0 days
Testing same since   103774  2016-12-20 14:02:26 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 

jobs:
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-amd64-amd64-xl-qemuu-debianhvm-i386 pass
 test-amd64-amd64-libvirt fail



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Not pushing.


commit 8867a48e058ab04bd269cab12ef4de5ee8ad5b7b
Author: Andrew Cooper 
Date:   Mon Dec 19 12:05:20 2016 +

x86/hvm: Don't emulate all instructions hitting the #UD intercept

Having the instruction emulator fill in all #UDs when using FEP is unhelpful
when trying to test emulation behaviour against hardware.

Restrict emulation from the #UD intercept to the cross-vendor case, and 
when a
postive Forced Emulation Prefix has been identified.

Signed-off-by: Andrew Cooper 
Reviewed-by: Jan Beulich 

commit eafc8ad471de861132b15e3268f72517c128a298
Author: Andrew Cooper 
Date:   Mon Dec 19 10:19:29 2016 +

x86/emul: Don't opencode CR0_TS in CLTS handling

Also replace implicit 0 checks with X86EMUL_OKAY

Signed-off-by: Andrew Cooper 
Reviewed-by: Jan Beulich 
(qemu changes not included)

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 1/7] Remove hardcoded strict -Werror checking

[Konrad Rzeszutek Wilk]

On Mon, Dec 19, 2016 at 09:53:02PM -0600, Doug Goldstein wrote:
> On 12/17/16 9:51 AM, Konrad Rzeszutek Wilk wrote:
> > On Fri, Dec 16, 2016 at 02:56:01PM -0800, Alistair Francis wrote:
> >> Signed-off-by: Alistair Francis 
> > 
> > 
> > Why?
> 
> *adjusts his distro maintainer hat* It's considered really bad form for
> upstreams to push -Werror in their projects. However I know there's
> upstream interest to keep it. The compromise would probably be to get my
> rear in gear and get a wider range of distros testing with Travis CI /
> GitLab CI.

So.. why not do something like this:


diff --git a/config/StdGNU.mk b/config/StdGNU.mk
index 6be8233..fd2d5b9 100644
--- a/config/StdGNU.mk
+++ b/config/StdGNU.mk
@@ -39,3 +39,5 @@ ifeq ($(lto),y)
 CFLAGS += -flto
 LDFLAGS-$(clang) += -plugin LLVMgold.so
 endif
+
+CFLAGS += $(EXTRA_CFLAGS)

And have the BuildRoot script do something like:


EXTRA_CFLAGS=$(CFLAGS)
unset CFLAGS

make -C xen

?

> 
> -- 
> Doug Goldstein
> 




___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen/evtchn: use rb_entry()

[Juergen Gross]

On 20/12/16 15:02, Geliang Tang wrote:
> To make the code clearer, use rb_entry() instead of container_of() to
> deal with rbtree.
> 
> Signed-off-by: Geliang Tang 

Reviewed-by: Juergen Gross 

> ---
>  drivers/xen/evtchn.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/xen/evtchn.c b/drivers/xen/evtchn.c
> index e8c7f09..6890897 100644
> --- a/drivers/xen/evtchn.c
> +++ b/drivers/xen/evtchn.c
> @@ -125,7 +125,7 @@ static int add_evtchn(struct per_user_data *u, struct 
> user_evtchn *evtchn)
>   while (*new) {
>   struct user_evtchn *this;
>  
> - this = container_of(*new, struct user_evtchn, node);
> + this = rb_entry(*new, struct user_evtchn, node);
>  
>   parent = *new;
>   if (this->port < evtchn->port)
> @@ -157,7 +157,7 @@ static struct user_evtchn *find_evtchn(struct 
> per_user_data *u, unsigned port)
>   while (node) {
>   struct user_evtchn *evtchn;
>  
> - evtchn = container_of(node, struct user_evtchn, node);
> + evtchn = rb_entry(node, struct user_evtchn, node);
>  
>   if (evtchn->port < port)
>   node = node->rb_left;
> 


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 05/13] pvh/acpi: Handle ACPI accesses for PVH guests

[Andrew Cooper]

On 20/12/2016 15:41, Jan Beulich wrote:
 On 20.12.16 at 16:29,  wrote:
>> On 12/20/2016 09:47 AM, Jan Beulich wrote:
>> On 20.12.16 at 15:35,  wrote:
 On 12/20/2016 06:50 AM, Jan Beulich wrote:
>> +else
>> +{
>> +uint32_t v = *val;
>> +
>> +/* Status register is write-1-to-clear by guests */
>> +switch ( port & 3 )
>> +{
>> +case 0:
>> +*sts &= ~(v & 0xff);
>> +*sts &= *mask_sts;
> I can understand the first &=, but why would a read have this second
> (side) effect? I could see some sort of need for such only when you
> were setting any flags.
 This is a write, not a read.
>>> Oh, right. But the question remains about that unexpected side
>>> effect.
>> It indeed doesn't do anything for the case of guest access. It is
>> guarding against setting unauthorized bits by domctl (introduced by the
>> next patch). And I can move it into that case.
>>
>> However, as discussed in the thread about 06/13 patch, we may currently
>> not need to provide domctl access to anything but VCPU map. So the
>> question is whether domctl interface to GPE0/PM1a is needed at all. I
>> think it's a useful interface with very little extra code required and
>> the toolstack can use it, for example, to inject events into guests. But
>> I don't have a specific use case.
> To be honest, I'd keep out anything we don't really need.

I have two specific use-cases.  1) DIMM Hotplug, and 2) Windows
tablet/desktop mode switch, both of which I think will just require a
toolstack entity to be able to raise an SCI.

However, I am happy for you to ignore these case for now (as they
haven't yet been fully designed and thought through), on the assumption
that if we do need to add anything, it will happily sit alongside the
VCPU code.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen/blkback: use rb_entry()

[Konrad Rzeszutek Wilk]

On Tue, Dec 20, 2016 at 10:02:19PM +0800, Geliang Tang wrote:
> To make the code clearer, use rb_entry() instead of container_of() to
> deal with rbtree.

That is OK but I think 'container_of' is more clear.

Roger, thoughts?

> 
> Signed-off-by: Geliang Tang 
> ---
>  drivers/block/xen-blkback/blkback.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/block/xen-blkback/blkback.c 
> b/drivers/block/xen-blkback/blkback.c
> index 726c32e..7e59fae 100644
> --- a/drivers/block/xen-blkback/blkback.c
> +++ b/drivers/block/xen-blkback/blkback.c
> @@ -191,10 +191,10 @@ static void make_response(struct xen_blkif_ring *ring, 
> u64 id,
> unsigned short op, int st);
>  
>  #define foreach_grant_safe(pos, n, rbtree, node) \
> - for ((pos) = container_of(rb_first((rbtree)), typeof(*(pos)), node), \
> + for ((pos) = rb_entry(rb_first((rbtree)), typeof(*(pos)), node), \
>(n) = (&(pos)->node != NULL) ? rb_next(&(pos)->node) : NULL; \
>&(pos)->node != NULL; \
> -  (pos) = container_of(n, typeof(*(pos)), node), \
> +  (pos) = rb_entry(n, typeof(*(pos)), node), \
>(n) = (&(pos)->node != NULL) ? rb_next(&(pos)->node) : NULL)
>  
>  
> @@ -223,7 +223,7 @@ static int add_persistent_gnt(struct xen_blkif_ring *ring,
>   /* Figure out where to put new node */
>   new = &ring->persistent_gnts.rb_node;
>   while (*new) {
> - this = container_of(*new, struct persistent_gnt, node);
> + this = rb_entry(*new, struct persistent_gnt, node);
>  
>   parent = *new;
>   if (persistent_gnt->gnt < this->gnt)
> @@ -254,7 +254,7 @@ static struct persistent_gnt *get_persistent_gnt(struct 
> xen_blkif_ring *ring,
>  
>   node = ring->persistent_gnts.rb_node;
>   while (node) {
> - data = container_of(node, struct persistent_gnt, node);
> + data = rb_entry(node, struct persistent_gnt, node);
>  
>   if (gref < data->gnt)
>   node = node->rb_left;
> -- 
> 2.9.3
> 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 05/13] pvh/acpi: Handle ACPI accesses for PVH guests

[Boris Ostrovsky]

On 12/20/2016 11:46 AM, Andrew Cooper wrote:
> On 20/12/2016 15:41, Jan Beulich wrote:
> On 20.12.16 at 16:29,  wrote:
>>> On 12/20/2016 09:47 AM, Jan Beulich wrote:
>>> On 20.12.16 at 15:35,  wrote:
> On 12/20/2016 06:50 AM, Jan Beulich wrote:
>>> +else
>>> +{
>>> +uint32_t v = *val;
>>> +
>>> +/* Status register is write-1-to-clear by guests */
>>> +switch ( port & 3 )
>>> +{
>>> +case 0:
>>> +*sts &= ~(v & 0xff);
>>> +*sts &= *mask_sts;
>> I can understand the first &=, but why would a read have this second
>> (side) effect? I could see some sort of need for such only when you
>> were setting any flags.
> This is a write, not a read.
 Oh, right. But the question remains about that unexpected side
 effect.
>>> It indeed doesn't do anything for the case of guest access. It is
>>> guarding against setting unauthorized bits by domctl (introduced by the
>>> next patch). And I can move it into that case.
>>>
>>> However, as discussed in the thread about 06/13 patch, we may currently
>>> not need to provide domctl access to anything but VCPU map. So the
>>> question is whether domctl interface to GPE0/PM1a is needed at all. I
>>> think it's a useful interface with very little extra code required and
>>> the toolstack can use it, for example, to inject events into guests. But
>>> I don't have a specific use case.
>> To be honest, I'd keep out anything we don't really need.
> I have two specific use-cases.  1) DIMM Hotplug, and 2) Windows
> tablet/desktop mode switch, both of which I think will just require a
> toolstack entity to be able to raise an SCI.

And I just found these two: XEN_DOMCTL_SENDTRIGGER_POWER and
XEN_DOMCTL_SENDTRIGGER_POWER.

When we switch HVM to new interface these can be re-implemented on top
of ACPI access.

-boris

>
> However, I am happy for you to ignore these case for now (as they
> haven't yet been fully designed and thought through), on the assumption
> that if we do need to add anything, it will happily sit alongside the
> VCPU code.
>
> ~Andrew


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] merlot0 and merlot1 (was Re: [xen-4.4-testing test] 103756: trouble: blocked/broken/fail/pass)

[Ian Jackson]

Jan Beulich writes ("[Xen-devel] merlot0 and merlot1 (was Re: [xen-4.4-testing 
test] 103756: trouble: blocked/broken/fail/pass)"):
> I consider this difficult to imagine, but it's also not entirely
> impossible. Since this appears to recur, are there perhaps logs
> (ideally more than one instance, to allow finding a possible
> pattern) from right before the issue occurred?

If it was software induced it was caused by

  
http://logs.test-lab.xenproject.org/osstest/logs/103741/test-xtf-amd64-amd64-3/info.html
  
http://logs.test-lab.xenproject.org/osstest/logs/103737/test-amd64-amd64-xl-xsm/info.html

(merlot0 and merlot1 respectively)

Both are Xen boot failures in xen-unstable.

The former failed with a Xen panic (see below).  The latter also with
a similar-looking panic.

I have noticed a tendency for boot order to be corrupted in situations
where Xen on the hard disk crashes on boot.  Or rather, when I find
that I am resetting the boot order, I disproportionately often find
that the host in question is in a crash/reboot loop.

Ian.


Dec 19 05:19:50.177998 (XEN) [ Xen-4.9-unstable  x86_64  debug=y   Not 
tainted ]
Dec 19 05:19:50.185993 (XEN) CPU:0
Dec 19 05:19:50.186022 (XEN) RIP:e008:[] 
__bitmap_and+0x24/0x33
Dec 19 05:19:50.194012 (XEN) RFLAGS: 00010202   CONTEXT: hypervisor
Dec 19 05:19:50.202015 (XEN) rax:    rbx: 82d08030fd58   
rcx: 0001
Dec 19 05:19:50.209997 (XEN) rdx: 82d0802edda0   rsi: 83023ffd1ef0   
rdi: 
Dec 19 05:19:50.217999 (XEN) rbp: 82d08030fd18   rsp: 82d08030fd18   
r8:  0004
Dec 19 05:19:50.226030 (XEN) r9:  82d080323a48   r10:    
r11: 
Dec 19 05:19:50.234016 (XEN) r12: 0028   r13: 83023ffd1ef0   
r14: 
Dec 19 05:19:50.242002 (XEN) r15: 82d080343aa0   cr0: 8005003b   
cr4: 000406e0
Dec 19 05:19:50.242044 (XEN) cr3: bdaff000   cr2: 
Dec 19 05:19:50.250003 (XEN) ds:    es:    fs:    gs:    ss: 
   cs: e008
Dec 19 05:19:50.258000 (XEN) Xen code around  
(__bitmap_and+0x24/0x33):
Dec 19 05:19:50.266001 (XEN)  48 8b 0c c2 48 23 0c c6 <48> 89 0c c7 48 83 c0 01 
41 39 c0 7f eb 5d c3 55
Dec 19 05:19:50.274021 (XEN) Xen stack trace from rsp=82d08030fd18:
Dec 19 05:19:50.274059 (XEN)82d08030fd48 82d0801719cc 
83023e203800 83023e384030
Dec 19 05:19:50.282033 (XEN)82d08030fd58  
82d08030fd88 82d080171bbc
Dec 19 05:19:50.290024 (XEN)  
82d08030fd88 83023e384010
Dec 19 05:19:50.298002 (XEN)82d080329b70 0038 
82d08030fdc8 82d0802afb8b
Dec 19 05:19:50.305991 (XEN)c000  
ffed 82d08030
Dec 19 05:19:50.313991 (XEN) 82d080343aa0 
82d08030fdd8 82d0802b00d7
Dec 19 05:19:50.321999 (XEN)82d08030fdf8 82d0802ac9cb 
82d080343aa0 83023fff5fe0
Dec 19 05:19:50.329995 (XEN)82d08030ff08 82d0802c0ff2 
 0010
Dec 19 05:19:50.337992 (XEN)01ff 0001 
0001 0001
Dec 19 05:19:50.345995 (XEN)0001 0001 
0001 0001
Dec 19 05:19:50.354022 (XEN) 011f3000 
0043efff 00800163
Dec 19 05:19:50.362005 (XEN)f001 00043ee0 
0002 8308dd80
Dec 19 05:19:50.369994 (XEN)8308def0 8308dfb0 
 
Dec 19 05:19:50.378000 (XEN)0008 0001006e 
0003 02f8
Dec 19 05:19:50.386046 (XEN)  
 
Dec 19 05:19:50.393995 (XEN)  
 82d080100073
Dec 19 05:19:50.401991 (XEN)  
 
Dec 19 05:19:50.410017 (XEN)  
 
Dec 19 05:19:50.417998 (XEN)  
 
Dec 19 05:19:50.426028 (XEN)  
 
Dec 19 05:19:50.434004 (XEN) Xen call trace:
Dec 19 05:19:50.434037 (XEN)[] __bitmap_and+0x24/0x33
Dec 19 05:19:50.442054 (XEN)[] msi_compose_msg+0x7e/0xf5
Dec 19 05:19:50.442092 (XEN)[] __setup_msi_irq+0x2f/0x5c
Dec 19 05:19:50.450046 (XEN)[] amd_iommu_init+0x460/0x6b7
Dec 19 05:19:50.457996 (XEN)[] amd_iov_detect+0x69/0xad
Dec 19 05:19:50.465982 (XEN)[] iommu_setup+0x61/0x1c1
Dec 19 05:19:50.466019 (XEN)[] __start_xen+0x23a8/0x2878
Dec 19 05:19:50.473994 (XEN)[] __high_start+0x53/0x55
Dec 19 05:19:50.481998 (XEN) 
Dec 19 05:19:50.482025 (XEN) Pagetable walk from :
Dec 19 05:1

Re: [Xen-devel] [PATCH 1/7] Remove hardcoded strict -Werror checking

[Doug Goldstein]

On 12/20/16 10:05 AM, Konrad Rzeszutek Wilk wrote:
> On Mon, Dec 19, 2016 at 09:53:02PM -0600, Doug Goldstein wrote:
>> On 12/17/16 9:51 AM, Konrad Rzeszutek Wilk wrote:
>>> On Fri, Dec 16, 2016 at 02:56:01PM -0800, Alistair Francis wrote:
 Signed-off-by: Alistair Francis 
>>>
>>>
>>> Why?
>>
>> *adjusts his distro maintainer hat* It's considered really bad form for
>> upstreams to push -Werror in their projects. However I know there's
>> upstream interest to keep it. The compromise would probably be to get my
>> rear in gear and get a wider range of distros testing with Travis CI /
>> GitLab CI.
> 
> So.. why not do something like this:
> 
> 
> diff --git a/config/StdGNU.mk b/config/StdGNU.mk
> index 6be8233..fd2d5b9 100644
> --- a/config/StdGNU.mk
> +++ b/config/StdGNU.mk
> @@ -39,3 +39,5 @@ ifeq ($(lto),y)
>  CFLAGS += -flto
>  LDFLAGS-$(clang) += -plugin LLVMgold.so
>  endif
> +
> +CFLAGS += $(EXTRA_CFLAGS)
> 
> And have the BuildRoot script do something like:
> 
> 
> EXTRA_CFLAGS=$(CFLAGS)
> unset CFLAGS
> 
> make -C xen
> 
> ?
> 

This was really to replace patch 3/7 right? Cause I think this would be
an ok change. It'd certainly help me with Yocto where I have to pass in
CFLAGS (need to pass in the sysroot of the target compiler).

-- 
Doug Goldstein



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [xen-unstable test] 103762: regressions - trouble: broken/fail/pass

[osstest service owner]

flight 103762 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/103762/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-xtf-amd64-amd64-33 host-install(3)broken REGR. vs. 103466
 test-xtf-amd64-amd64-43 host-install(3)broken REGR. vs. 103466
 test-amd64-i386-qemut-rhel6hvm-amd  3 host-install(3)  broken REGR. vs. 103466
 test-amd64-i386-qemuu-rhel6hvm-amd  3 host-install(3)  broken REGR. vs. 103466
 test-amd64-amd64-xl-qemuu-win7-amd64 3 host-install(3) broken REGR. vs. 103466
 test-amd64-amd64-xl   3 host-install(3)broken REGR. vs. 103466
 test-amd64-amd64-i386-pvgrub  3 host-install(3)broken REGR. vs. 103466
 test-amd64-amd64-xl-xsm   3 host-install(3)broken REGR. vs. 103466
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 3 host-install(3) broken REGR. vs. 
103466
 test-amd64-i386-xl-qemuu-ovmf-amd64  3 host-install(3) broken REGR. vs. 103466
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 3 host-install(3) broken 
REGR. vs. 103466
 test-armhf-armhf-libvirt-raw  9 debian-di-installfail REGR. vs. 103394
 test-amd64-amd64-qemuu-nested-intel 16 debian-hvm-install/l1/l2 fail REGR. vs. 
103466

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-rtds  3 host-install(3)broken REGR. vs. 103466
 test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail  like 103466
 test-armhf-armhf-libvirt-qcow2 12 saverestore-support-check   fail like 103466
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stopfail like 103466
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 103466
 test-armhf-armhf-libvirt 13 saverestore-support-checkfail  like 103466
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 103466

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  7469686ccc959765542cd10551f9bd7a602f2cbd
baseline version:
 xen  fc9229c4bb35c5474fbc67f78e628dcbcc90afc5

Last test of basis   103466  2016-12-16 13:24:29 Z4 days
Failing since103540  2016-12-17 16:30:58 Z3 days6 attempts
Testing same since   103762  2016-12-20 01:16:40 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Anshul Makkar 
  Bhupinder Thakur 
  Boris Ostrovsky 
  Cédric Bosdonnat 
  Daniel De Graaf 
  Edgar E. Iglesias 
  Haozhong Zhang 
  Ian Jackson 
  Jan Beulich 
  Juergen Gross 
  Julien Grall 
  Kevin Tian 
  Konrad Rzeszutek Wilk 
  Razvan Cojocaru 
  Ross Lagerwall 
  Stefano Stabellini 
 

Re: [Xen-devel] [PATCH 1/7] Remove hardcoded strict -Werror checking

[Konrad Rzeszutek Wilk]

On Tue, Dec 20, 2016 at 11:02:15AM -0600, Doug Goldstein wrote:
> On 12/20/16 10:05 AM, Konrad Rzeszutek Wilk wrote:
> > On Mon, Dec 19, 2016 at 09:53:02PM -0600, Doug Goldstein wrote:
> >> On 12/17/16 9:51 AM, Konrad Rzeszutek Wilk wrote:
> >>> On Fri, Dec 16, 2016 at 02:56:01PM -0800, Alistair Francis wrote:
>  Signed-off-by: Alistair Francis 
> >>>
> >>>
> >>> Why?
> >>
> >> *adjusts his distro maintainer hat* It's considered really bad form for
> >> upstreams to push -Werror in their projects. However I know there's
> >> upstream interest to keep it. The compromise would probably be to get my
> >> rear in gear and get a wider range of distros testing with Travis CI /
> >> GitLab CI.
> > 
> > So.. why not do something like this:
> > 
> > 
> > diff --git a/config/StdGNU.mk b/config/StdGNU.mk
> > index 6be8233..fd2d5b9 100644
> > --- a/config/StdGNU.mk
> > +++ b/config/StdGNU.mk
> > @@ -39,3 +39,5 @@ ifeq ($(lto),y)
> >  CFLAGS += -flto
> >  LDFLAGS-$(clang) += -plugin LLVMgold.so
> >  endif
> > +
> > +CFLAGS += $(EXTRA_CFLAGS)
> > 
> > And have the BuildRoot script do something like:
> > 
> > 
> > EXTRA_CFLAGS=$(CFLAGS)
> > unset CFLAGS
> > 
> > make -C xen
> > 
> > ?
> > 
> 
> This was really to replace patch 3/7 right? Cause I think this would be
> an ok change. It'd certainly help me with Yocto where I have to pass in
> CFLAGS (need to pass in the sysroot of the target compiler).

Yes sorry.
> 
> -- 
> Doug Goldstein
> 




___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 08/10] x86/vm-event: use unambiguous register names

[Tamas K Lengyel]

2016-12-20 3:42 GMT-07:00 Jan Beulich :

> This is in preparation of eliminating the mis-naming of 64-bit fields
> with 32-bit register names (eflags instead of rflags etc).
>
> Signed-off-by: Jan Beulich 
>

 Acked-by: Tamas K Lengyel 
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 11/14] xen/x86: parse Dom0 kernel for PVHv2

[Roger Pau Monne]

On Fri, Dec 09, 2016 at 10:05:18AM -0700, Jan Beulich wrote:
> >>> On 30.11.16 at 17:49,  wrote:
> > @@ -1930,12 +1931,148 @@ static int __init hvm_setup_p2m(struct domain *d)
> >  #undef MB1_PAGES
> >  }
> >  
> > +static int __init hvm_copy_to_phys(struct domain *d, paddr_t paddr, void 
> > *buf,
> > +   int size)
> 
> I guess you made size plain int because hvm_copy_to_guest_phys()
> has it that way, but please let's not spread such bogus things - sizes
> can't possibly be negative.
> 
> > +{
> > +struct vcpu *saved_current;
> > +int rc;
> > +
> > +saved_current = current;
> > +set_current(d->vcpu[0]);
> > +rc = hvm_copy_to_guest_phys(paddr, buf, size);
> > +set_current(saved_current);
> 
> I continue to be uncertain about the behavior of this if something
> inside hvm_copy_to_guest_phys() goes wrong: Did you either
> statically analyze the code or try in practice out whether the
> playing with current makes understanding the crash output any
> harder?

If you managed to somehow call hvm_copy_to_guest_phys with the idle vcpu as
current you would get this kind of error, which I admin is maybe not that
obvious (apart from the IDLEv0 prefix).

(XEN) IDLEv0 Error pfn 21bd: rd=32767 od=32756 caf=180 
taf=

See below.

> While there's going to be some work involved with it, I do think
> that the use here might be a reason for the whole hvm_copy()
> machinery to gain a struct vcpu* parameter.

I've gone that route and added a new param to __hvm_copy, and also introduced
hvm_copy_to_guest_phys_vcpu which takes an additional vcpu parameter. While
there I've also added an assert to __hvm_copy in order to make sure the
vcpu parameter is always a hvm/pvh vcpu.

> > +static int __init hvm_load_kernel(struct domain *d, const module_t *image,
> > +  unsigned long image_headroom,
> > +  module_t *initrd, char *image_base,
> > +  char *cmdline, paddr_t *entry,
> > +  paddr_t *start_info_addr)
> > +{
> > +char *image_start = image_base + image_headroom;
> > +unsigned long image_len = image->mod_end;
> > +struct elf_binary elf;
> > +struct elf_dom_parms parms;
> > +paddr_t last_addr;
> > +struct hvm_start_info start_info;
> > +struct hvm_modlist_entry mod;
> > +struct vcpu *saved_current, *v = d->vcpu[0];
> > +int rc;
> > +
> > +if ( (rc = bzimage_parse(image_base, &image_start, &image_len)) != 0 )
> > +{
> > +printk("Error trying to detect bz compressed kernel\n");
> > +return rc;
> > +}
> > +
> > +if ( (rc = elf_init(&elf, image_start, image_len)) != 0 )
> > +{
> > +printk("Unable to init ELF\n");
> > +return rc;
> > +}
> > +#ifdef VERBOSE
> > +elf_set_verbose(&elf);
> > +#endif
> > +elf_parse_binary(&elf);
> > +if ( (rc = elf_xen_parse(&elf, &parms)) != 0 )
> > +{
> > +printk("Unable to parse kernel for ELFNOTES\n");
> > +return rc;
> > +}
> > +
> > +if ( parms.phys_entry == UNSET_ADDR32 ) {
> > +printk("Unable to find XEN_ELFNOTE_PHYS32_ENTRY address\n");
> > +return -EINVAL;
> > +}
> > +
> > +printk("OS: %s version: %s loader: %s bitness: %s\n", parms.guest_os,
> > +   parms.guest_ver, parms.loader,
> > +   elf_64bit(&elf) ? "64-bit" : "32-bit");
> > +
> > +/* Copy the OS image and free temporary buffer. */
> > +elf.dest_base = (void *)(parms.virt_kstart - parms.virt_base);
> > +elf.dest_size = parms.virt_kend - parms.virt_kstart;
> > +
> > +saved_current = current;
> > +set_current(v);
> > +rc = elf_load_binary(&elf);
> > +set_current(saved_current);
> 
> Same reservations as above.

Right, this one however is more tricky to fix since elf_load_binary is shared
with libxc, so adding a vcpu/domain parameter here is problematic for the
toolstack side. That's quite similar to what happens on classic PV Dom0
creation, we need to switch to Dom0 page tables. I'm not trying to use that to
justify that this is the best way, but everything else seems quite convoluted
(either adding a new param to elf_load_binary or a new field to struct
elf_binary in order to store the domain/vcpu).

> > +if ( rc < 0 )
> > +{
> > +printk("Failed to load kernel: %d\n", rc);
> > +printk("Xen dom0 kernel broken ELF: %s\n", elf_check_broken(&elf));
> > +return rc;
> > +}
> > +
> > +last_addr = ROUNDUP(parms.virt_kend - parms.virt_base, PAGE_SIZE);
> > +
> > +if ( initrd != NULL )
> > +{
> > +rc = hvm_copy_to_phys(d, last_addr, mfn_to_virt(initrd->mod_start),
> > +  initrd->mod_end);
> > +if ( rc )
> > +{
> > +printk("Unable to copy initrd to guest\n");
> > +return rc;
> > +}
> > +
> > +mod.paddr = last_ad

Re: [Xen-devel] [PATCH 00/10] x86: register renaming (part I)

[Andrew Cooper]

On 20/12/2016 09:55, Jan Beulich wrote:
> This is a first (of three, as far as current plans go) steps to do away
> with misleading register names (eax instead of rax).
>
> 01: x86/MSR: introduce MSR access split/fold helpers
> 02: x86/guest-walk: use unambiguous register names
> 03: x86/shadow: use unambiguous register names
> 04: x86/oprofile: use unambiguous register names
> 05: x86/HVM: use unambiguous register names
> 06: x86/HVMemul: use unambiguous register names
> 07: x86/SVM: use unambiguous register names
> (VMX counterpart omitted for now, as I'll need to re-base)
> 08: x86/vm-event: use unambiguous register names
> 09: x86/traps: use unambiguous register names
> 10: x86/misc: use unambiguous register names
>
> Signed-off-by: Jan Beulich 
>

I haven't looked at these carefully, but they all seem to be sensible
mechanical changes, so

Acked-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 05/10] x86/HVM: use unambiguous register names

[Andrew Cooper]

On 20/12/2016 10:39, Jan Beulich wrote:
> @@ -3032,16 +3032,16 @@ void hvm_task_switch(
>  if ( hvm_set_cr3(tss.cr3, 1) )
>  goto out;
>  
> -regs->eip= tss.eip;
> -regs->eflags = tss.eflags | 2;
> -regs->eax= tss.eax;
> -regs->ecx= tss.ecx;
> -regs->edx= tss.edx;
> -regs->ebx= tss.ebx;
> -regs->esp= tss.esp;
> -regs->ebp= tss.ebp;
> -regs->esi= tss.esi;
> -regs->edi= tss.edi;
> +regs->rip= tss.eip;
> +regs->rflags = tss.eflags | 2;

As you are modifying this anyway, mind avoiding this opencoding?

~Andrew

> +regs->rax= tss.eax;
> +regs->rcx= tss.ecx;
> +regs->rdx= tss.edx;
> +regs->rbx= tss.ebx;
> +regs->rsp= tss.esp;
> +regs->rbp= tss.ebp;
> +regs->rsi= tss.esi;
> +regs->rdi= tss.edi;
>  
>  exn_raised = 0;
>  if ( hvm_load_segment_selector(x86_seg_es, tss.es, tss.eflags) ||
>

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [RFC 2/7] xen: Move xen files to accel/

[Eduardo Habkost]

Cc: Stefano Stabellini 
Cc: Anthony Perard 
Cc: xen-de...@lists.xensource.com
Signed-off-by: Eduardo Habkost 
---
 Makefile.target| 4 +---
 xen-common.c => accel/xen-common.c | 0
 xen-hvm.c => accel/xen-hvm.c   | 0
 xen-mapcache.c => accel/xen-mapcache.c | 0
 MAINTAINERS| 1 -
 accel/Makefile.objs| 2 ++
 6 files changed, 3 insertions(+), 4 deletions(-)
 rename xen-common.c => accel/xen-common.c (100%)
 rename xen-hvm.c => accel/xen-hvm.c (100%)
 rename xen-mapcache.c => accel/xen-mapcache.c (100%)
 create mode 100644 accel/Makefile.objs

diff --git a/Makefile.target b/Makefile.target
index 7c2fda9..0b2ad86 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -144,9 +144,7 @@ obj-y += dump.o
 obj-y += migration/ram.o migration/savevm.o
 LIBS := $(libs_softmmu) $(LIBS)
 
-# xen support
-obj-$(CONFIG_XEN) += xen-common.o
-obj-$(CONFIG_XEN_I386) += xen-hvm.o xen-mapcache.o
+obj-y += accel/
 
 # Hardware support
 ifeq ($(TARGET_NAME), sparc64)
diff --git a/xen-common.c b/accel/xen-common.c
similarity index 100%
rename from xen-common.c
rename to accel/xen-common.c
diff --git a/xen-hvm.c b/accel/xen-hvm.c
similarity index 100%
rename from xen-hvm.c
rename to accel/xen-hvm.c
diff --git a/xen-mapcache.c b/accel/xen-mapcache.c
similarity index 100%
rename from xen-mapcache.c
rename to accel/xen-mapcache.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 4a60579..8e25438 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -311,7 +311,6 @@ M: Stefano Stabellini 
 M: Anthony Perard 
 L: xen-de...@lists.xensource.com
 S: Supported
-F: xen-*
 F: */xen*
 F: hw/char/xen_console.c
 F: hw/display/xenfb.c
diff --git a/accel/Makefile.objs b/accel/Makefile.objs
new file mode 100644
index 000..6f3630e
--- /dev/null
+++ b/accel/Makefile.objs
@@ -0,0 +1,2 @@
+obj-$(CONFIG_XEN) += xen-common.o
+obj-$(CONFIG_XEN_I386) += xen-hvm.o xen-mapcache.o
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [RFC 0/7] Move accel, KVM, Xen, qtest files to accel/ subdir

[Eduardo Habkost]

This moves the KVM and Xen files to the an accel/ subdir.

Instead of moving the *-stubs.c file to accel/ as-is, I tried to
move most of the stub code to libqemustub.a. This way the obj-y
logic for accel/ is simpler: obj-y includes accel/ only if
CONFIG_SOFTMMU is set.

The Xen stubs could be moved completely to stubs/, but some of
the KVM stubs depend on cpu.h. So most of the kvm-stub.c code was
moved to stubs/kvm.c, but some of that code was kept in
accel/kvm-stub.c.

About TCG:
--

It is not obvious to me which TCG-related files could be moved to
accel/, so this series don't move any of them yet.

About other CONFIG_SOFTMMU top-level files:
---

I would like to know what we should do with the top-level
CONFIG_SOFTMMU-only files that don't belong to hw/. Some
candidates: arch_init.c cpus.c monitor.c gdbstub.c balloon.c
ioport.c bootdevice.c memory.c cputlb.c memory_mapping.c dump.c.

Maybe a sysemu/ subdir? In that case, should we still create an
accel/ subdir, or move xen-*, kvm-* and friends to sysemu/ too?

Cc: Paolo Bonzini 
Cc: k...@vger.kernel.org
Cc: Christoffer Dall 
Cc: Anthony Perard 
Cc: Stefano Stabellini 
Cc: xen-de...@lists.xensource.com

Eduardo Habkost (7):
  xen: Move xen-*-stub.c to stubs/
  xen: Move xen files to accel/
  kvm: Move some kvm-stub.c code to stubs/kvm.c
  kvm: Include kvm-stub.o only on CONFIG_SOFTMMU
  kvm: Move kvm*.c files to accel/
  accel: Move accel.c to accel/
  accel: Move qtest.c to accel/

 Makefile.objs  |  2 +-
 Makefile.target| 10 ++
 accel.c => accel/accel.c   |  0
 kvm-all.c => accel/kvm-common.c|  0
 kvm-stub.c => accel/kvm-stub.c | 51 --
 qtest.c => accel/qtest.c   |  0
 xen-common.c => accel/xen-common.c |  0
 xen-hvm.c => accel/xen-hvm.c   |  0
 xen-mapcache.c => accel/xen-mapcache.c |  0
 stubs/kvm.c| 65 ++
 xen-hvm-stub.c => stubs/xen-hvm.c  |  0
 xen-common-stub.c => stubs/xen.c   |  0
 MAINTAINERS|  4 +--
 accel/Makefile.objs|  9 +
 stubs/Makefile.objs|  2 ++
 15 files changed, 80 insertions(+), 63 deletions(-)
 rename accel.c => accel/accel.c (100%)
 rename kvm-all.c => accel/kvm-common.c (100%)
 rename kvm-stub.c => accel/kvm-stub.c (71%)
 rename qtest.c => accel/qtest.c (100%)
 rename xen-common.c => accel/xen-common.c (100%)
 rename xen-hvm.c => accel/xen-hvm.c (100%)
 rename xen-mapcache.c => accel/xen-mapcache.c (100%)
 rename xen-hvm-stub.c => stubs/xen-hvm.c (100%)
 rename xen-common-stub.c => stubs/xen.c (100%)
 create mode 100644 accel/Makefile.objs

-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen/blkback: use rb_entry()

[Roger Pau Monné]

On Tue, Dec 20, 2016 at 11:47:03AM -0500, Konrad Rzeszutek Wilk wrote:
> On Tue, Dec 20, 2016 at 10:02:19PM +0800, Geliang Tang wrote:
> > To make the code clearer, use rb_entry() instead of container_of() to
> > deal with rbtree.
> 
> That is OK but I think 'container_of' is more clear.
> 
> Roger, thoughts?

I think so, container_of is a global macro that's widely used and everyone
knows, rb_entry OTOH it's not and it's use doesn't really simply the code at
all. I'm not really opposed, but it seems kind of a pointless change (not that
it's wrong).

Roger.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Xen ARM community call - meeting minutes and date for the next one

[Andrii Anisov]

Julien, Stefano,

Are there any updates about:


ACTION: Bosch to send a bug report regarding xen-swiotlb

Edgar:  IOMMU could not be used by the guest (Stage-1). This would be useful
to implement driver in userspace.
Julien: When will it be required?
Edgar:  It is a trend


Any mailing threads or discussions?

--

*Andrii Anisov*

*Lead Systems Engineer*

*Office: *+380 44 390 5457  *x* 66766 
*Cell: *+380 50 5738852 *Email: 
*andrii_ani...@epam.com 


*Kyiv**,* *Ukraine *(GMT+3)*epam.com *

CONFIDENTIALITY CAUTION AND DISCLAIMER
This message is intended only for the use of the individual(s) or 
entity(ies) to which it is addressed and contains information that is 
legally privileged and confidential. If you are not the intended 
recipient, or the person responsible for delivering the message to the 
intended recipient, you are hereby notified that any dissemination, 
distribution or copying of this communication is strictly prohibited. 
All unintended recipients are obliged to delete this message and destroy 
any printed copies.



___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [RFC 1/7] xen: Move xen-*-stub.c to stubs/

[Eduardo Habkost]

Move xen stubs to stubs/ so they are handled automatically by
libqemustub.a.

Cc: Stefano Stabellini 
Cc: Anthony Perard 
Cc: xen-de...@lists.xensource.com
Cc: Paolo Bonzini 
Signed-off-by: Eduardo Habkost 
---
 Makefile.target   | 2 --
 xen-hvm-stub.c => stubs/xen-hvm.c | 0
 xen-common-stub.c => stubs/xen.c  | 0
 stubs/Makefile.objs   | 2 ++
 4 files changed, 2 insertions(+), 2 deletions(-)
 rename xen-hvm-stub.c => stubs/xen-hvm.c (100%)
 rename xen-common-stub.c => stubs/xen.c (100%)

diff --git a/Makefile.target b/Makefile.target
index 7a5080e..7c2fda9 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -147,8 +147,6 @@ LIBS := $(libs_softmmu) $(LIBS)
 # xen support
 obj-$(CONFIG_XEN) += xen-common.o
 obj-$(CONFIG_XEN_I386) += xen-hvm.o xen-mapcache.o
-obj-$(call lnot,$(CONFIG_XEN)) += xen-common-stub.o
-obj-$(call lnot,$(CONFIG_XEN_I386)) += xen-hvm-stub.o
 
 # Hardware support
 ifeq ($(TARGET_NAME), sparc64)
diff --git a/xen-hvm-stub.c b/stubs/xen-hvm.c
similarity index 100%
rename from xen-hvm-stub.c
rename to stubs/xen-hvm.c
diff --git a/xen-common-stub.c b/stubs/xen.c
similarity index 100%
rename from xen-common-stub.c
rename to stubs/xen.c
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 2b5bb74..227e154 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -50,3 +50,5 @@ stub-obj-y += smbios_type_38.o
 stub-obj-y += ipmi.o
 stub-obj-y += pc_madt_cpu_entry.o
 stub-obj-y += migration-colo.o
+stub-obj-y += xen.o
+stub-obj-y += xen-hvm.o
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 4/7] tools: Update sys/poll.h to poll.h

[Alistair Francis]

On Mon, Dec 19, 2016 at 8:00 PM, Doug Goldstein  wrote:
> On 12/19/16 12:01 PM, Alistair Francis wrote:
>> On Sat, Dec 17, 2016 at 7:55 AM, Konrad Rzeszutek Wilk
>>  wrote:
>>> On Fri, Dec 16, 2016 at 02:56:04PM -0800, Alistair Francis wrote:
 To avoid this build error with newer build systems:
>>>
>>> And what is newer? GCC 5? 6?
>>
>> In this case it is GCC 5.
>>
>
> No. Its a libc thing. I wonder what libc you're using because glibc 2.24
> still has it at sys/pool.h for me.

Yeah, you are right. I don't know what I was thinking.

I'm using musl version 1.1.15 which prints a warning every time you
call sys/poll.h.

Looking at the latest glibc master branch glibc supports both and
doesn't have any warnings. This probably isn't the right thing to do
then. I will just have to remove Werror instead.

Thanks,

Alistair

>
> --
> Doug Goldstein
>
>
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
>

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen/blkback: use rb_entry()

[Konrad Rzeszutek Wilk]

On Tue, Dec 20, 2016 at 05:44:06PM +, Roger Pau Monné wrote:
> On Tue, Dec 20, 2016 at 11:47:03AM -0500, Konrad Rzeszutek Wilk wrote:
> > On Tue, Dec 20, 2016 at 10:02:19PM +0800, Geliang Tang wrote:
> > > To make the code clearer, use rb_entry() instead of container_of() to
> > > deal with rbtree.
> > 
> > That is OK but I think 'container_of' is more clear.
> > 
> > Roger, thoughts?
> 
> I think so, container_of is a global macro that's widely used and everyone
> knows, rb_entry OTOH it's not and it's use doesn't really simply the code at
> all. I'm not really opposed, but it seems kind of a pointless change (not that
> it's wrong).

 I concur.

Geliang Tang,

Thank you for the patch but there is no need for it.

Thanks again!
> 
> Roger.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 1/7] Remove hardcoded strict -Werror checking

[Alistair Francis]

On Tue, Dec 20, 2016 at 9:21 AM, Konrad Rzeszutek Wilk
 wrote:
> On Tue, Dec 20, 2016 at 11:02:15AM -0600, Doug Goldstein wrote:
>> On 12/20/16 10:05 AM, Konrad Rzeszutek Wilk wrote:
>> > On Mon, Dec 19, 2016 at 09:53:02PM -0600, Doug Goldstein wrote:
>> >> On 12/17/16 9:51 AM, Konrad Rzeszutek Wilk wrote:
>> >>> On Fri, Dec 16, 2016 at 02:56:01PM -0800, Alistair Francis wrote:
>>  Signed-off-by: Alistair Francis 
>> >>>
>> >>>
>> >>> Why?
>> >>
>> >> *adjusts his distro maintainer hat* It's considered really bad form for
>> >> upstreams to push -Werror in their projects. However I know there's
>> >> upstream interest to keep it. The compromise would probably be to get my
>> >> rear in gear and get a wider range of distros testing with Travis CI /
>> >> GitLab CI.
>> >
>> > So.. why not do something like this:
>> >
>> >
>> > diff --git a/config/StdGNU.mk b/config/StdGNU.mk
>> > index 6be8233..fd2d5b9 100644
>> > --- a/config/StdGNU.mk
>> > +++ b/config/StdGNU.mk
>> > @@ -39,3 +39,5 @@ ifeq ($(lto),y)
>> >  CFLAGS += -flto
>> >  LDFLAGS-$(clang) += -plugin LLVMgold.so
>> >  endif
>> > +
>> > +CFLAGS += $(EXTRA_CFLAGS)
>> >
>> > And have the BuildRoot script do something like:
>> >
>> >
>> > EXTRA_CFLAGS=$(CFLAGS)
>> > unset CFLAGS
>> >
>> > make -C xen
>> >
>> > ?
>> >
>>
>> This was really to replace patch 3/7 right? Cause I think this would be
>> an ok change. It'd certainly help me with Yocto where I have to pass in
>> CFLAGS (need to pass in the sysroot of the target compiler).
>
> Yes sorry.

Yeah, this could work as well (for patch 3).

Should I re-spin with something like that?

Thanks,

Alistair

>>
>> --
>> Doug Goldstein
>>
>
>
>

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 1/7] Remove hardcoded strict -Werror checking

[Alistair Francis]

On Mon, Dec 19, 2016 at 7:53 PM, Doug Goldstein  wrote:
> On 12/17/16 9:51 AM, Konrad Rzeszutek Wilk wrote:
>> On Fri, Dec 16, 2016 at 02:56:01PM -0800, Alistair Francis wrote:
>>> Signed-off-by: Alistair Francis 
>>
>>
>> Why?
>
> *adjusts his distro maintainer hat* It's considered really bad form for
> upstreams to push -Werror in their projects. However I know there's
> upstream interest to keep it. The compromise would probably be to get my
> rear in gear and get a wider range of distros testing with Travis CI /
> GitLab CI.

I agree with both points. I understand why upstream wants it there,
but it's a pain.

What about an option to override and remove Werror, so it can still be
enabled by default?

Thanks,

Alistair

>
> --
> Doug Goldstein
>

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel