Re: [Xen-devel] [PATCH v3] x86/apicv: Enhance posted-interrupt processing

[Tian, Kevin]

> From: Gao, Chao
> Sent: Thursday, March 02, 2017 12:28 PM
> 
> On Thu, Mar 02, 2017 at 02:41:55AM -0700, Jan Beulich wrote:
>  On 02.03.17 at 02:49,  wrote:
> >> +if ( cpu != smp_process_id() )
> >
> >Did you not even build test this patch? I don't think the construct
> >above compiles.
> 
> Really sorry. I forgot to commit after I fixed this.
> Acturally, I found this, fixed it and tested this patch several times( with or
> without VT-d PI) to avoid some obvious regression.
> 

I'm OK with the code change with above line being fixed. Since I'll
take vacation next week, I can give my ack here instead of being
bottle-neck given likely next version is only for comment change
(hope Jan will help ack that part):

Acked-by: Kevin Tian 

of course if next version has any functional change, then please
ignore this ack. :-)

Thanks
Kevin

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

Re: [Xen-devel] [PATCH 1/2] x86/hvm: Don't raise #GP behind the emulators back for CR accesses

[Tian, Kevin]

> From: Andrew Cooper [mailto:andrew.coop...@citrix.com]
> Sent: Thursday, March 02, 2017 11:00 PM
> 
> hvm_set_cr{0,4}() are reachable from the emulator, but use
> hvm_inject_hw_exception() directly.
> 
> Alter the API to make the callers of hvm_set_cr{0,3,4}() responsible for
> raising #GP, and apply this change to all existing callers.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Kevin Tian 

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

Re: [Xen-devel] [PATCH v1 1/2] x86/vvmx: check vmcs address in vmread/vmwrite

[Tian, Kevin]

> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: Wednesday, March 01, 2017 11:40 PM
> 
> >>> On 01.03.17 at 16:23,  wrote:
> > On Wed, 2017-03-01 at 07:28 -0700, Jan Beulich wrote:
> >> > > > On 01.03.17 at 15:22,  wrote:
> >> >
> >> > On Wed, 2017-03-01 at 07:04 -0700, Jan Beulich wrote:
> >> > > > > > On 01.03.17 at 14:44,  wrote:
> >> > > >
> >> > > > Additionally, it would be painful to return the correct error value
> >> > > > all the way back to nvmx_handle_vmptrld().
> >> > >
> >> > > Surely that's at best relevant in the other patch. Here you're in
> >> > > virtual_vmcs_vmwrite_safe(), which already knows how to
> >> > > communicate back an error indicator.
> >> >
> >> > How checking the return value of virtual_vmcs_enter() is different
> >> > from checking nv_vvmcxaddr?
> >>
> >> Checking the address is just one step. As the other patch shows,
> >> checking the ID is necessary too. Over time more such checks may
> >> be found necessary. Checking what hardware hands us is a single
> >> check, and is always going to be sufficient no matter what new
> >> features get added to hardware.
> >
> > Conceptually, the result of __vmptrld() is irrelevant to a guest
> > during nested vmread/vmwrite emulation.  The fact that Xen is doing
> > __vmptrld() is a side effect of nested VMX.
> 
> True.
> 
> > All necessary checks may be found in Intel SDM.
> 
> All _currently_ necessary checks. It is precisely possible new ones
> getting added which I'd like to see covered by other than adding
> further checks to our software when hardware already does them.
> 
> >  And it states that
> > there can be only 1 VMfail if VMCS pointer is not valid: VMfailInvalid.
> > vmptrld can end up in 3 different VMfails and returning them to the
> > guest as a result of vmread/vmwrite emulation is wrong.
> 
> As is crashing Xen because of such.
> 
> The implication of course would be that the insn-error may need
> adjustment in such a case.
> 
> > (Even though each of them will end up being VMfailInvalid in current
> > implementation)
> >
> > I think that Xen's goal in nested VMX is emulating H/W as close as
> > possible.
> 
> Correct. Preferably by leveraging hardware instead of re-doing the
> same thing in software.
> 
> Anyway - we'll see what the VMX maintainers think.
> 

Although leveraging HW check is a generally good idea, I buy-in 
Sergey's comment that we may emulate different VMX feature set 
to guest in the future then in such case we'll need both SW/HW 
checks and then may still need to track latest SDM change.

So:

Acked-by: Kevin Tian 

Thanks
Kevin

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

[Xen-devel] [xen-unstable test] 106383: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl-multivcpu 15 guest-start/debian.repeat fail REGR. vs. 
106351

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail  like 106351
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stopfail like 106351
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 106351
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 106351
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 106351
 test-armhf-armhf-libvirt-raw 12 saverestore-support-checkfail  like 106351
 test-armhf-armhf-libvirt 13 saverestore-support-checkfail  like 106351
 test-amd64-amd64-xl-rtds  9 debian-install   fail  like 106351

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-rtds  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 build-arm64   5 xen-buildfail   never pass
 build-arm64-xsm   5 xen-buildfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 build-arm64-pvops 5 kernel-build fail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail 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-armhf-armhf-libvirt-xsm 12 migrate-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-raw 11 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 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-arndale  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  26735f30dffe1091686bbe921aacbea8ba371cc8
baseline version:
 xen  1a0ab02e342cfd80decd72606c94479e4b309a3c

Last test of basis   106351  2017-03-02 04:21:56 Z1 days
Failing since106371  2017-03-02 14:14:36 Z0 days2 attempts
Testing same since   106383  2017-03-02 21:45:05 Z0 days1 attempts


People who touched revisions under test:
  Anthony PERARD 
  Ian Jackson 
  Jan Beulich 
  Juergen Gross 
  Kevin Tian 
  Marek Marczykowski-Górecki 
  Olaf Hering 
  Wei Liu 

jobs:
 build-amd64-xsm  pas

Re: [Xen-devel] [PATCH v1 2/2] x86/vvmx: add vmcs id check into vmptrld emulation

[Tian, Kevin]

> From: Sergey Dyasli [mailto:sergey.dya...@citrix.com]
> Sent: Wednesday, March 01, 2017 5:14 PM
> 
> If a guest will do vmptrld with an incorrect vmcs id:
> 
> (XEN) Xen BUG at .../git/upstream/xen/xen/include/asm/hvm/vmx/vmx.h:333
> (XEN) [ Xen-4.9-unstable  x86_64  debug=y   Tainted:H ]
> (XEN) Xen call trace:
> (XEN)[]
> vmcs.c#arch/x86/hvm/vmx/vmcs.o.unlikely+0x28/0x19a
> (XEN)[] virtual_vmcs_vmread+0x11/0x2c
> (XEN)[] vvmx.c#_map_io_bitmap+0x86/0x88
> (XEN)[] nvmx_handle_vmptrld+0xf0/0x1fb
> (XEN)[] vmx_vmexit_handler+0x132b/0x1c49
> (XEN)[] vmx_asm_vmexit_handler+0x3c/0x120
> 
> Fix this by adding appropriate checks for vmcs id during vmptrld
> emulation.
> 
> Signed-off-by: Sergey Dyasli 

Acked-by: Kevin Tian 

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

Re: [Xen-devel] [RFC PATCH] mm, hotplug: get rid of auto_online_blocks

[Michal Hocko]

On Thu 02-03-17 18:03:15, Igor Mammedov wrote:
> On Thu, 2 Mar 2017 15:28:16 +0100
> Michal Hocko  wrote:
> 
> > On Thu 02-03-17 14:53:48, Igor Mammedov wrote:
> > [...]
> > > When trying to support memory unplug on guest side in RHEL7,
> > > experience shows otherwise. Simplistic udev rule which onlines
> > > added block doesn't work in case one wants to online it as movable.
> > > 
> > > Hotplugged blocks in current kernel should be onlined in reverse
> > > order to online blocks as movable depending on adjacent blocks zone.  
> > 
> > Could you be more specific please? Setting online_movable from the udev
> > rule should just work regardless of the ordering or the state of other
> > memblocks. If that doesn't work I would call it a bug.
> It's rather an implementation constrain than a bug
> for details and workaround patch see
>  [1] https://bugzilla.redhat.com/show_bug.cgi?id=1314306#c7

"You are not authorized to access bug #1314306"

could you paste the reasoning here please?

> patch attached there is limited by another memory hotplug
> issue, which is NORMAL/MOVABLE zone balance, if kernel runs
> on configuration where the most of memory is hot-removable
> kernel might experience lack of memory in zone NORMAL.

yes and that is an inherent problem of movable memory.

> > > Which means simple udev rule isn't usable since it gets event from
> > > the first to the last hotplugged block order. So now we would have
> > > to write a daemon that would
> > >  - watch for all blocks in hotplugged memory appear (how would it know)
> > >  - online them in right order (order might also be different depending
> > >on kernel version)
> > >-- it becomes even more complicated in NUMA case when there are
> > >   multiple zones and kernel would have to provide user-space
> > >   with information about zone maps
> > > 
> > > In short current experience shows that userspace approach
> > >  - doesn't solve issues that Vitaly has been fixing (i.e. onlining
> > >fast and/or under memory pressure) when udev (or something else
> > >might be killed)  
> > 
> > yeah and that is why the patch does the onlining from the kernel.
> onlining in this patch is limited to hyperv and patch breaks
> auto-online on x86 kvm/vmware/baremetal as they reuse the same
> hotplug path.

Those can use the udev or do you see any reason why they couldn't?

> > > > Can you imagine any situation when somebody actually might want to have
> > > > this knob enabled? From what I understand it doesn't seem to be the
> > > > case.  
> > > For x86:
> > >  * this config option is enabled by default in recent Fedora,  
> > 
> > How do you want to support usecases which really want to online memory
> > as movable? Do you expect those users to disable the option because
> > unless I am missing something the in kernel auto onlining only supporst
> > regular onlining.
>
> current auto onlining config option does what it's been designed for,
> i.e. it onlines hotplugged memory.
> It's possible for non average Fedora user to override default
> (commit 86dd995d6) if she/he needs non default behavior
> (i.e. user knows how to online manually and/or can write
> a daemon that would handle all of nuances of kernel in use).
> 
> For the rest when Fedora is used in cloud and user increases memory
> via management interface of whatever cloud she/he uses, it just works.
> 
> So it's choice of distribution to pick its own default that makes
> majority of user-base happy and this patch removes it without taking
> that in consideration.

You still can have a udev rule to achive the same thing for
non-ballooning based hotplug.

> How to online memory is different issue not related to this patch,
> current default onlining as ZONE_NORMAL works well for scaling
> up VMs.
> 
> Memory unplug is rather new and it doesn't work reliably so far,
> moving onlining to user-space won't really help. Further work
> is need to be done so that it would work reliably.

The main problem I have with this is that this is a limited usecase
driven configuration knob which doesn't work properly for other usecases
(namely movable online once your distribution choses to set the config
option to auto online). There is a userspace solution for this so this
shouldn't have been merged in the first place! It sneaked a proper review
process (linux-api wasn't CC to get a broader attenttion) which is
really sad.

So unless this causes a major regression which would be hard to fix I
will submit the patch for inclusion.
-- 
Michal Hocko
SUSE Labs

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

Re: [Xen-devel] [PATCH v9 1/8] VMX: Permanently assign PI hook vmx_pi_switch_to()

[Tian, Kevin]

> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: Wednesday, March 01, 2017 3:42 PM
> 
> >>> On 01.03.17 at 01:01,  wrote:
> > On Tue, Feb 28, 2017 at 09:43:09AM -0700, Jan Beulich wrote:
> > On 27.02.17 at 02:45,  wrote:
> >>> --- a/xen/arch/x86/hvm/vmx/vmx.c
> >>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> >>> @@ -260,9 +260,15 @@ void vmx_pi_hooks_deassign(struct domain *d)
> >>>
> >>>  ASSERT(d->arch.hvm_domain.pi_ops.vcpu_block);
> >>>
> >>> +/*
> >>> + * Note that we don't set 'd->arch.hvm_domain.pi_ops.switch_to' to 
> >>> NULL
> >>> + * here. If we deassign the hooks while the vCPU is runnable in the
> >>> + * runqueue with 'SN' set, all the future notification event will be
> >>> + * suppressed. Preserving the 'switch_to' hook function can make sure
> >>> + * event time the vCPU is running the 'SN' field is cleared.
> >>> + */
> >>
> >>Did we now lose the remark indicating that at least in theory
> >>we could remove the hook after it had run one more time? It's
> >
> > I also think it only need to run one more time, because the hook
> > function that set 'SN' bit is already removed.
> >
> >>also not really becoming clear why SN continues to matter
> >>after device removal, but perhaps that's just because of my
> >>so far limited understanding of PI.
> >>
> >>Also I think you mean "every time" on the last line, albeit that
> >>(as per my remark above) is irrelevant - we need it to run just
> >>once more.
> >
> > I would like to make the comment as clear as possible.
> > How about the underlying comment,
> >
> > Note that we don't set 'd->arch.hvm_domain.pi_ops.switch_to' to NULL
> > here. If we deassign the hooks while the vCPU is runnable in the
> > runqueue with 'SN' set, all the future notification event will be
> > suppressed since vmx_deliver_posted_intr() also use 'SN' bit
> > as the suppression flag. Preserving the 'switch_to' hook function can
> > clear the 'SN' bit when the vCPU becomes running next time. After
> > that, No matter which status(runnable, running or block) the vCPU is in,
> > the 'SN' bit will keep clear for the 'switch_from' hook function that set
> > the 'SN' bit has been removed. At that time, the 'switch_to' hook function
> > is also useless. Considering the function doesn't do harm to the whole
> > system, leave it here until we find a clean solution to deassign the
> > 'switch_to' hook function.
> 
> Sounds good to me (read: Reviewed-by: Jan Beulich ).
> If Kevin would give his ack, I could replace the comment while committing,
> so you wouldn't need to re-send.
> 
> Jan

Good to me too.

Acked-by: Kevin Tian 

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

[Xen-devel] [libvirt test] 106394: tolerable FAIL - PUSHED

[osstest service owner]

flight 106394 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/106394/

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-libvirt 13 saverestore-support-checkfail  like 106352
 test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail  like 106352
 test-armhf-armhf-libvirt-raw 12 saverestore-support-checkfail  like 106352

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt  1 build-check(1)   blocked  n/a
 build-arm64   5 xen-buildfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 build-arm64-xsm   5 xen-buildfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 build-arm64-pvops 5 kernel-build fail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass

version targeted for testing:
 libvirt  215a8a976466117104d216f1f336c2c3ad51d010
baseline version:
 libvirt  9d87f769726bd5714eb6a930379d8b4c53311196

Last test of basis   106352  2017-03-02 04:21:48 Z1 days
Testing same since   106394  2017-03-03 04:20:40 Z0 days1 attempts


People who touched revisions under test:
  Peter Krempa 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  fail
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  fail
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-arm64-libvirt  blocked 
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-arm64-pvopsfail
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-libvirt-xsm pass
 test-arm64-arm64-libvirt-xsm blocked 
 test-armhf-armhf-libvirt-xsm pass
 test-amd64-i386-libvirt-xsm  pass
 test-amd64-amd64-libvirt pass
 test-arm64-arm64-libvirt blocked 
 test-armhf-armhf-libvirt pass
 test-amd64-i386-libvirt  pass
 test-amd64-amd64-libvirt-pairpass
 test-amd64-i386-libvirt-pair pass
 test-arm64-arm64-libvirt-qcow2   blocked 
 test-armhf-armhf-libvirt-raw pass
 test-amd64-amd64-libvirt-vhd pass



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

Logs, config files, etc. are available at
http://logs.test-lab.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

[Xen-devel] [PATCH] tools/xenstore: define off_t

[Olaf Hering]

talloc.h uses off_t, but did not include 
Fixes 7012548e0d ("xenstore: enhance control command support")

Signed-off-by: Olaf Hering 
---
 tools/xenstore/talloc.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/xenstore/talloc.h b/tools/xenstore/talloc.h
index c849bf61af..71a36e7be0 100644
--- a/tools/xenstore/talloc.h
+++ b/tools/xenstore/talloc.h
@@ -24,6 +24,8 @@
License along with this library; If not, see .
 */
 
+#include 
+
 /* this is only needed for compatibility with the old talloc */
 typedef void TALLOC_CTX;
 

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

Re: [Xen-devel] [PATCH] tools/xenstore: define off_t

[Wei Liu]

On Fri, Mar 03, 2017 at 08:52:09AM +, Olaf Hering wrote:
> talloc.h uses off_t, but did not include 
> Fixes 7012548e0d ("xenstore: enhance control command support")

I think this issue is exposed by that particular commit. That commit is
fine in itself. So I would simply delete the "Fixes ..." part.


> 
> Signed-off-by: Olaf Hering 

Acked-by: Wei Liu 

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

Re: [Xen-devel] [PATCH] xen-netback: fix race condition on XenBus disconnect

[Paul Durrant]

> -Original Message-
> From: Igor Druzhinin [mailto:igor.druzhi...@citrix.com]
> Sent: 02 March 2017 22:57
> To: net...@vger.kernel.org; xen-de...@lists.xenproject.org
> Cc: Paul Durrant ; jgr...@suse.com; Wei Liu
> ; Igor Druzhinin 
> Subject: [PATCH] xen-netback: fix race condition on XenBus disconnect
> 
> In some cases during XenBus disconnect event handling and subsequent
> queue resource release there may be some TX handlers active on
> other processors. Use RCU in order to synchronize with them.
> 
> Signed-off-by: Igor Druzhinin 
> ---
>  drivers/net/xen-netback/interface.c | 13 -
>  drivers/net/xen-netback/xenbus.c| 17 +++--
>  2 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-
> netback/interface.c
> index a2d32676..32e2cc6 100644
> --- a/drivers/net/xen-netback/interface.c
> +++ b/drivers/net/xen-netback/interface.c
> @@ -164,7 +164,7 @@ static int xenvif_start_xmit(struct sk_buff *skb, struct
> net_device *dev)
>  {
>   struct xenvif *vif = netdev_priv(dev);
>   struct xenvif_queue *queue = NULL;
> - unsigned int num_queues = vif->num_queues;

Do you not need an rcu_read_lock() around this and use of the num_queues value 
(as you have below)?

> + unsigned int num_queues = rcu_dereference(vif)->num_queues;
>   u16 index;
>   struct xenvif_rx_cb *cb;
> 
> @@ -221,18 +221,21 @@ static struct net_device_stats
> *xenvif_get_stats(struct net_device *dev)
>  {
>   struct xenvif *vif = netdev_priv(dev);
>   struct xenvif_queue *queue = NULL;
> + unsigned int num_queues;
>   u64 rx_bytes = 0;
>   u64 rx_packets = 0;
>   u64 tx_bytes = 0;
>   u64 tx_packets = 0;
>   unsigned int index;
> 
> - spin_lock(&vif->lock);
> - if (vif->queues == NULL)
> + rcu_read_lock();
> +
> + num_queues = rcu_dereference(vif)->num_queues;
> + if (num_queues < 1)
>   goto out;
> 
>   /* Aggregate tx and rx stats from each queue */
> - for (index = 0; index < vif->num_queues; ++index) {
> + for (index = 0; index < num_queues; ++index) {
>   queue = &vif->queues[index];
>   rx_bytes += queue->stats.rx_bytes;
>   rx_packets += queue->stats.rx_packets;
> @@ -241,7 +244,7 @@ static struct net_device_stats
> *xenvif_get_stats(struct net_device *dev)
>   }
> 
>  out:
> - spin_unlock(&vif->lock);
> + rcu_read_unlock();
> 
>   vif->dev->stats.rx_bytes = rx_bytes;
>   vif->dev->stats.rx_packets = rx_packets;
> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-
> netback/xenbus.c
> index d2d7cd9..76efb01 100644
> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -495,26 +495,23 @@ static void backend_disconnect(struct
> backend_info *be)
>   struct xenvif *vif = be->vif;
> 
>   if (vif) {
> + unsigned int num_queues = vif->num_queues;
>   unsigned int queue_index;
> - struct xenvif_queue *queues;
> 
>   xen_unregister_watchers(vif);
>  #ifdef CONFIG_DEBUG_FS
>   xenvif_debugfs_delif(vif);
>  #endif /* CONFIG_DEBUG_FS */
>   xenvif_disconnect_data(vif);
> - for (queue_index = 0;
> -  queue_index < vif->num_queues;
> -  ++queue_index)
> - xenvif_deinit_queue(&vif->queues[queue_index]);
> 
> - spin_lock(&vif->lock);
> - queues = vif->queues;
>   vif->num_queues = 0;
> - vif->queues = NULL;
> - spin_unlock(&vif->lock);
> + synchronize_net();

So, num_queues is your RCU protected value, rather than the queues pointer, in 
which case I think you probably need to change code such as

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/net/xen-netback/netback.c?id=refs/tags/v4.10#n216

to be gated on num_queues.

Also shouldn't xenvif_up(), xenvif_down() and xenvif_get_ethtool_stats() not be 
using rcu_read_lock() and rcu_dereference() of num_queues as well?

  Paul

> 
> - vfree(queues);
> + for (queue_index = 0; queue_index < num_queues;
> ++queue_index)
> + xenvif_deinit_queue(&vif->queues[queue_index]);
> +
> + vfree(vif->queues);
> + vif->queues = NULL;
> 
>   xenvif_disconnect_ctrl(vif);
>   }
> --
> 1.8.3.1


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

Re: [Xen-devel] [PATCH 1/6] xen: credit1: simplify csched_runq_steal() a little bit.

[anshul makkar]

On 02/03/17 10:38, Dario Faggioli wrote:

Since we're holding the lock on the pCPU from which we
are trying to steal, it can't have disappeared, so we
can drop the check for that (and convert it in an
ASSERT()).

And since we try to steal only from busy pCPUs, it's
unlikely for such pCPU to be idle, so we mark it as
such (and bail early if it unfortunately is).

Signed-off-by: Dario Faggioli 
---
Cc: George Dunlap 
---
  xen/common/sched_credit.c |   87 +++--
  1 file changed, 44 insertions(+), 43 deletions(-)

diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 4649e64..63a8675 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -1593,64 +1593,65 @@ static struct csched_vcpu *
  csched_runq_steal(int peer_cpu, int cpu, int pri, int balance_step)
  {
  const struct csched_pcpu * const peer_pcpu = CSCHED_PCPU(peer_cpu);
-const struct vcpu * const peer_vcpu = curr_on_cpu(peer_cpu);
  struct csched_vcpu *speer;
  struct list_head *iter;
  struct vcpu *vc;
  
+ASSERT(peer_pcpu != NULL);

+
  /*
   * Don't steal from an idle CPU's runq because it's about to
   * pick up work from it itself.
   */
-if ( peer_pcpu != NULL && !is_idle_vcpu(peer_vcpu) )
+if ( unlikely(is_idle_vcpu(curr_on_cpu(peer_cpu))) )
+goto out;
We can just use if (!is_idle_vcpu(peer_vcpu)). Why to replace it with 
some code that introduces an unnecessary branch statement.

+
+list_for_each( iter, &peer_pcpu->runq )
  {
-list_for_each( iter, &peer_pcpu->runq )
-{
-speer = __runq_elem(iter);
+speer = __runq_elem(iter);
  
-/*

- * If next available VCPU here is not of strictly higher
- * priority than ours, this PCPU is useless to us.
- */
-if ( speer->pri <= pri )
-break;
+/*
+ * If next available VCPU here is not of strictly higher
+ * priority than ours, this PCPU is useless to us.
+ */
+if ( speer->pri <= pri )
+break;
  
-/* Is this VCPU runnable on our PCPU? */

-vc = speer->vcpu;
-BUG_ON( is_idle_vcpu(vc) );
+/* Is this VCPU runnable on our PCPU? */
+vc = speer->vcpu;
+BUG_ON( is_idle_vcpu(vc) );
  
-/*

- * If the vcpu has no useful soft affinity, skip this vcpu.
- * In fact, what we want is to check if we have any "soft-affine
- * work" to steal, before starting to look at "hard-affine work".
- *
- * Notice that, if not even one vCPU on this runq has a useful
- * soft affinity, we could have avoid considering this runq for
- * a soft balancing step in the first place. This, for instance,
- * can be implemented by taking note of on what runq there are
- * vCPUs with useful soft affinities in some sort of bitmap
- * or counter.
- */
-if ( balance_step == CSCHED_BALANCE_SOFT_AFFINITY
- && !__vcpu_has_soft_affinity(vc, vc->cpu_hard_affinity) )
-continue;
+/*
+ * If the vcpu has no useful soft affinity, skip this vcpu.
+ * In fact, what we want is to check if we have any "soft-affine
+ * work" to steal, before starting to look at "hard-affine work".
+ *
+ * Notice that, if not even one vCPU on this runq has a useful
+ * soft affinity, we could have avoid considering this runq for
+ * a soft balancing step in the first place. This, for instance,
+ * can be implemented by taking note of on what runq there are
+ * vCPUs with useful soft affinities in some sort of bitmap
+ * or counter.
+ */
Isn't it a better approach that now as we have came across a vcpu which 
doesn't have a desired soft affinity but is a potential candidate for 
migration, so instead of just forgetting it,  lets save the information 
for such vcpus in some data structure in some order so that we don't 
have to scan them again if we don't find anything useful in the present run.

+if ( balance_step == CSCHED_BALANCE_SOFT_AFFINITY
+ && !__vcpu_has_soft_affinity(vc, vc->cpu_hard_affinity) )
+continue;
  
-csched_balance_cpumask(vc, balance_step, cpumask_scratch);

-if ( __csched_vcpu_is_migrateable(vc, cpu, cpumask_scratch) )
-{
-/* We got a candidate. Grab it! */
-TRACE_3D(TRC_CSCHED_STOLEN_VCPU, peer_cpu,
- vc->domain->domain_id, vc->vcpu_id);
-SCHED_VCPU_STAT_CRANK(speer, migrate_q);
-SCHED_STAT_CRANK(migrate_queued);
-WARN_ON(vc->is_urgent);
-__runq_remove(speer);
-vc->processor = cpu;
-return speer;
-  

[Xen-devel] [PATCH v4] x86/apicv: Fix wrong IPI suppression during posted interrupt delivery

[Chao Gao]

__vmx_deliver_posted_interrupt() wrongly used a softirq bit to decide whether
to suppress an IPI. Its logic was: the first time an IPI was sent, we set
the softirq bit. Next time, we would check that softirq bit before sending
another IPI. If the 1st IPI arrived at the pCPU which was in
non-root mode, the hardware would consume the IPI and sync PIR to vIRR.
During the process, no one (both hardware and software) will clear the
softirq bit. As a result, the following IPI would be wrongly suppressed.

This patch discards the suppression check, always sending an IPI.
The softirq also need to be raised. But there is a little change.
This patch moves the place where we raise a softirq for
'cpu != smp_processor_id()' case to the IPI interrupt handler.
Namely, don't raise a softirq for this case and set the interrupt handler
to pi_notification_interrupt()(in which a softirq is raised) regardless of
VT-d PI enabled or not. The only difference is when an IPI arrives at the
pCPU which is happened in non-root mode, the code will not raise a useless
softirq since the IPI is consumed by hardware rather than raise a softirq
unconditionally.

Signed-off-by: Quan Xu 
Signed-off-by: Chao Gao 
Acked-by: Kevin Tian 
---
v4:
- Replace patch v3 title "Enhance posted-interrupt processing" with the
current title.
- Desciption changes
- Fix a compiler error

---
 xen/arch/x86/hvm/vmx/vmx.c | 50 +++---
 1 file changed, 43 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 5b1717d..1a0e130 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1842,13 +1842,53 @@ static void __vmx_deliver_posted_interrupt(struct vcpu 
*v)
 bool_t running = v->is_running;
 
 vcpu_unblock(v);
+/*
+ * Just like vcpu_kick(), nothing is needed for the following two cases:
+ * 1. The target vCPU is not running, meaning it is blocked or runnable.
+ * 2. The target vCPU is the current vCPU and we're in non-interrupt
+ * context.
+ */
 if ( running && (in_irq() || (v != current)) )
 {
+/*
+ * Note: Only two cases will reach here:
+ * 1. The target vCPU is running on other pCPU.
+ * 2. The target vCPU is the current vCPU.
+ *
+ * Note2: Don't worry the v->processor may change. The vCPU being
+ * moved to another processor is guaranteed to sync PIR to vIRR,
+ * due to the involved scheduling cycle.
+ */
 unsigned int cpu = v->processor;
 
-if ( !test_and_set_bit(VCPU_KICK_SOFTIRQ, &softirq_pending(cpu))
- && (cpu != smp_processor_id()) )
+/*
+ * For case 1, we send an IPI to the pCPU. When an IPI arrives, the
+ * target vCPU maybe is running in non-root mode, running in root
+ * mode, runnable or blocked. If the target vCPU is running in
+ * non-root mode, the hardware will sync PIR to vIRR for
+ * 'posted_intr_vector' is special to the pCPU. If the target vCPU is
+ * running in root-mode, the interrupt handler starts to run.
+ * Considering an IPI may arrive in the window between the call to
+ * vmx_intr_assist() and interrupts getting disabled, the interrupt
+ * handler should raise a softirq to ensure events will be delivered
+ * in time. If the target vCPU is runnable, it will sync PIR to
+ * vIRR next time it is chose to run. In this case, a IPI and a
+ * softirq is sent to a wrong vCPU which will not have any adverse
+ * effect. If the target vCPU is blocked, since vcpu_block() checks
+ * whether there is an event to be delivered through
+ * local_events_need_delivery() just after blocking, the vCPU must
+ * have synced PIR to vIRR. Similarly, there is a IPI and a softirq
+ * sent to a wrong vCPU.
+ */
+if ( cpu != smp_processor_id() )
 send_IPI_mask(cpumask_of(cpu), posted_intr_vector);
+/*
+ * For case 2, raising a softirq ensures PIR will be synced to vIRR.
+ * As any softirq will do, as an optimization we only raise one if
+ * none is pending already.
+ */
+else if ( !softirq_pending(cpu) )
+raise_softirq(VCPU_KICK_SOFTIRQ);
 }
 }
 
@@ -2281,13 +2321,9 @@ const struct hvm_function_table * __init start_vmx(void)
 
 if ( cpu_has_vmx_posted_intr_processing )
 {
+alloc_direct_apic_vector(&posted_intr_vector, 
pi_notification_interrupt);
 if ( iommu_intpost )
-{
-alloc_direct_apic_vector(&posted_intr_vector, 
pi_notification_interrupt);
 alloc_direct_apic_vector(&pi_wakeup_vector, pi_wakeup_interrupt);
-}
-else
-alloc_direct_apic_vector(&posted_intr_vector, 
event_check_interrupt);
 }
 else
 {
-- 
1.8.3.1


___
Xen-devel mailing list

[Xen-devel] [PATCH RFC 5/5] x86: clean up header files in domain_build.c

[Wei Liu]

Remove the ones that are no longer needed and sort them.

Signed-off-by: Wei Liu 
---
 xen/arch/x86/domain_build.c | 40 ++--
 1 file changed, 6 insertions(+), 34 deletions(-)

diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
index 64a978473d..e66aacdad6 100644
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -5,45 +5,17 @@
  */
 
 #include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
 #include 
-#include 
-#include 
 #include 
 #include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
 #include 
-#include 
-#include 
 #include 
-#include  /* for bzimage_parse */
-#include 
-#include 
-
-#include 
-
-#include 
-#include 
-#include 
-#include 
 
 #include "domain_build.h"
 
-- 
2.11.0


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

[Xen-devel] [PATCH RFC 2/5] xen: include xen/types.h in domain.h

[Wei Liu]

The public header expects a few types to be present.

This works in the code base only because types.h is included by some
other headers which happen to be placed before the inclusion of
domain.h.

Include types.h before xen.h in domain.h to fix it properly.

Signed-off-by: Wei Liu 
---
 xen/include/xen/domain.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index bce0ea1ea9..347f264047 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -2,6 +2,8 @@
 #ifndef __XEN_DOMAIN_H__
 #define __XEN_DOMAIN_H__
 
+#include 
+
 #include 
 #include 
 #include 
-- 
2.11.0


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

[Xen-devel] [PATCH RFC 4/5] x86: split PVH dom0 builder to domain_build_pv.c

[Wei Liu]

Long term we want to be able to disentangle PV and HVM code. Move the
PVH domain builder to a dedicated file. It will later depends on
CONFIG_HVM or CONFIG_PVH.

This in turn requires exposing a few functions and variables via
domain_build.h.

Signed-off-by: Wei Liu 
---
 xen/arch/x86/Makefile   |1 +
 xen/arch/x86/domain_build.c | 1068 +
 xen/arch/x86/domain_build.h |8 +
 xen/arch/x86/domain_build_pvh.c | 1097 +++
 4 files changed, 1108 insertions(+), 1066 deletions(-)
 create mode 100644 xen/arch/x86/domain_build_pvh.c

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index e4c134e66a..2ad94f50a8 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -25,6 +25,7 @@ obj-y += domctl.o
 obj-y += domain.o
 obj-bin-y += domain_build.init.o
 obj-bin-y += domain_build_pv.init.o
+obj-bin-y += domain_build_pvh.init.o
 obj-y += domain_page.o
 obj-y += e820.o
 obj-y += extable.o
diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
index 4cabe2fb2e..64a978473d 100644
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -51,27 +51,6 @@ static long __initdata dom0_nrpages;
 static long __initdata dom0_min_nrpages;
 static long __initdata dom0_max_nrpages = LONG_MAX;
 
-/*
- * Have the TSS cover the ISA port range, which makes it
- * - 104 bytes base structure
- * - 32 bytes interrupt redirection bitmap
- * - 128 bytes I/O bitmap
- * - one trailing byte
- * or a total of 265 bytes.
- *
- * NB: as PVHv2 Dom0 doesn't have legacy devices (ISA), it shouldn't have any
- * business in accessing the ISA port range, much less in real mode, and due to
- * the lack of firmware it shouldn't also execute any INT instruction. This is
- * done just for consistency with what hvmloader does.
- */
-#define HVM_VM86_TSS_SIZE 265
-
-static unsigned int __initdata acpi_intr_overrides;
-static struct acpi_madt_interrupt_override __initdata *intsrcovr;
-
-static unsigned int __initdata acpi_nmi_sources;
-static struct acpi_madt_nmi_source __initdata *nmisrc;
-
 /*
  * dom0_mem=[min:,][max:,][]
  * 
@@ -262,8 +241,8 @@ boolean_param("ro-hpet", ro_hpet);
 
 unsigned int __initdata memflags = MEMF_no_dma|MEMF_exact_node;
 
-static unsigned long __init dom0_paging_pages(const struct domain *d,
-  unsigned long nr_pages)
+unsigned long __init dom0_paging_pages(const struct domain *d,
+   unsigned long nr_pages)
 {
 /* Copied from: libxl_get_required_shadow_memory() */
 unsigned long memkb = nr_pages * (PAGE_SIZE / 1024);
@@ -492,1049 +471,6 @@ int __init setup_permissions(struct domain *d)
 return rc;
 }
 
-
-static int __init modify_identity_mmio(struct domain *d, unsigned long pfn,
-   unsigned long nr_pages, const bool map)
-{
-int rc;
-
-for ( ; ; )
-{
-rc = (map ? map_mmio_regions : unmap_mmio_regions)
- (d, _gfn(pfn), nr_pages, _mfn(pfn));
-if ( rc == 0 )
-break;
-if ( rc < 0 )
-{
-printk(XENLOG_WARNING
-   "Failed to identity %smap [%#lx,%#lx) for d%d: %d\n",
-   map ? "" : "un", pfn, pfn + nr_pages, d->domain_id, rc);
-break;
-}
-nr_pages -= rc;
-pfn += rc;
-process_pending_softirqs();
-}
-
-return rc;
-}
-
-/* Populate a HVM memory range using the biggest possible order. */
-static int __init pvh_populate_memory_range(struct domain *d,
-unsigned long start,
-unsigned long nr_pages)
-{
-unsigned int order, i = 0;
-struct page_info *page;
-int rc;
-#define MAP_MAX_ITER 64
-
-order = MAX_ORDER;
-while ( nr_pages != 0 )
-{
-unsigned int range_order = get_order_from_pages(nr_pages + 1);
-
-order = min(range_order ? range_order - 1 : 0, order);
-page = alloc_domheap_pages(d, order, memflags);
-if ( page == NULL )
-{
-if ( order == 0 && memflags )
-{
-/* Try again without any memflags. */
-memflags = 0;
-order = MAX_ORDER;
-continue;
-}
-if ( order == 0 )
-{
-printk("Unable to allocate memory with order 0!\n");
-return -ENOMEM;
-}
-order--;
-continue;
-}
-
-rc = guest_physmap_add_page(d, _gfn(start), _mfn(page_to_mfn(page)),
-order);
-if ( rc != 0 )
-{
-printk("Failed to populate memory: [%#lx,%lx): %d\n",
-   start, start + (1UL << order), rc);
-return -ENOMEM;
-}
-start += 1UL << order;
-nr_pages -= 1UL << order;
-if ( (++i % MAP_M

[Xen-devel] [PATCH RFC 3/5] x86: split PV dom0 builder to domain_build_pv.c

[Wei Liu]

Long term we want to be able to disentangle PV and HVM code. Move the PV
domain builder to a dedicated file.

This in turn requires exposing a few functions and variables via
a new header domain_build.h.

Signed-off-by: Wei Liu 
---
 xen/arch/x86/Makefile  |   1 +
 xen/arch/x86/domain_build.c| 894 +---
 xen/arch/x86/domain_build.h|  32 ++
 xen/arch/x86/domain_build_pv.c | 909 +
 4 files changed, 951 insertions(+), 885 deletions(-)
 create mode 100644 xen/arch/x86/domain_build.h
 create mode 100644 xen/arch/x86/domain_build_pv.c

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index d6980b563d..e4c134e66a 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -24,6 +24,7 @@ obj-bin-y += dmi_scan.init.o
 obj-y += domctl.o
 obj-y += domain.o
 obj-bin-y += domain_build.init.o
+obj-bin-y += domain_build_pv.init.o
 obj-y += domain_page.o
 obj-y += e820.o
 obj-y += extable.o
diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
index 00ce5f24e9..4cabe2fb2e 100644
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -45,6 +45,8 @@
 #include 
 #include 
 
+#include "domain_build.h"
+
 static long __initdata dom0_nrpages;
 static long __initdata dom0_min_nrpages;
 static long __initdata dom0_max_nrpages = LONG_MAX;
@@ -154,11 +156,11 @@ static void __init parse_dom0_nodes(const char *s)
 }
 custom_param("dom0_nodes", parse_dom0_nodes);
 
-static cpumask_t __initdata dom0_cpus;
+cpumask_t __initdata dom0_cpus;
 
-static struct vcpu *__init setup_dom0_vcpu(struct domain *d,
-   unsigned int vcpu_id,
-   unsigned int cpu)
+struct vcpu *__init setup_dom0_vcpu(struct domain *d,
+unsigned int vcpu_id,
+unsigned int cpu)
 {
 struct vcpu *v = alloc_vcpu(d, vcpu_id, cpu);
 
@@ -258,66 +260,7 @@ string_param("dom0_ioports_disable", 
opt_dom0_ioports_disable);
 static bool_t __initdata ro_hpet = 1;
 boolean_param("ro-hpet", ro_hpet);
 
-/* Allow ring-3 access in long mode as guest cannot use ring 1 ... */
-#define BASE_PROT (_PAGE_PRESENT|_PAGE_RW|_PAGE_ACCESSED|_PAGE_USER)
-#define L1_PROT (BASE_PROT|_PAGE_GUEST_KERNEL)
-/* ... except for compatibility mode guests. */
-#define COMPAT_L1_PROT (_PAGE_PRESENT|_PAGE_RW|_PAGE_ACCESSED)
-#define L2_PROT (BASE_PROT|_PAGE_DIRTY)
-#define L3_PROT (BASE_PROT|_PAGE_DIRTY)
-#define L4_PROT (BASE_PROT|_PAGE_DIRTY)
-
-static unsigned int __initdata memflags = MEMF_no_dma|MEMF_exact_node;
-
-static struct page_info * __init alloc_chunk(
-struct domain *d, unsigned long max_pages)
-{
-static unsigned int __initdata last_order = MAX_ORDER;
-struct page_info *page;
-unsigned int order = get_order_from_pages(max_pages), free_order;
-
-if ( order > last_order )
-order = last_order;
-else if ( max_pages & (max_pages - 1) )
---order;
-while ( (page = alloc_domheap_pages(d, order, memflags)) == NULL )
-if ( order-- == 0 )
-break;
-if ( page )
-last_order = order;
-else if ( memflags )
-{
-/*
- * Allocate up to 2MB at a time: It prevents allocating very large
- * chunks from DMA pools before the >4GB pool is fully depleted.
- */
-last_order = 21 - PAGE_SHIFT;
-memflags = 0;
-return alloc_chunk(d, max_pages);
-}
-
-/*
- * Make a reasonable attempt at finding a smaller chunk at a higher
- * address, to avoid allocating from low memory as much as possible.
- */
-for ( free_order = order; !memflags && page && order--; )
-{
-struct page_info *pg2;
-
-if ( d->tot_pages + (1 << order) > d->max_pages )
-continue;
-pg2 = alloc_domheap_pages(d, order, MEMF_exact_node);
-if ( pg2 > page )
-{
-free_domheap_pages(page, free_order);
-page = pg2;
-free_order = order;
-}
-else if ( pg2 )
-free_domheap_pages(pg2, order);
-}
-return page;
-}
+unsigned int __initdata memflags = MEMF_no_dma|MEMF_exact_node;
 
 static unsigned long __init dom0_paging_pages(const struct domain *d,
   unsigned long nr_pages)
@@ -330,7 +273,7 @@ static unsigned long __init dom0_paging_pages(const struct 
domain *d,
 return ((memkb + 1023) / 1024) << (20 - PAGE_SHIFT);
 }
 
-static unsigned long __init compute_dom0_nr_pages(
+unsigned long __init compute_dom0_nr_pages(
 struct domain *d, struct elf_dom_parms *parms, unsigned long initrd_len)
 {
 nodeid_t node;
@@ -467,199 +410,7 @@ static void __init process_dom0_ioports_disable(struct 
domain *dom0)
 }
 }
 
-static __init void dom0_update_physmap(struct domain *d, unsigned long pfn,
-   unsigned long mfn, unsigned

[Xen-devel] [PATCH RFC 1/5] xen: move round_pg{up,down} to pfn.h

[Wei Liu]

They are going to be needed in multiple places. Instead of replicating
more, move them to pfn.h.

Signed-off-by: Wei Liu 
---
 xen/arch/x86/domain_build.c | 3 ---
 xen/common/page_alloc.c | 4 +---
 xen/include/xen/pfn.h   | 3 +++
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
index e4b60d436f..00ce5f24e9 100644
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -267,9 +267,6 @@ boolean_param("ro-hpet", ro_hpet);
 #define L3_PROT (BASE_PROT|_PAGE_DIRTY)
 #define L4_PROT (BASE_PROT|_PAGE_DIRTY)
 
-#define round_pgup(_p)(((_p)+(PAGE_SIZE-1))&PAGE_MASK)
-#define round_pgdown(_p)  ((_p)&PAGE_MASK)
-
 static unsigned int __initdata memflags = MEMF_no_dma|MEMF_exact_node;
 
 static struct page_info * __init alloc_chunk(
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 530ede1d20..42c20cb201 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -131,6 +131,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -176,9 +177,6 @@ size_param("bootscrub_chunk", opt_bootscrub_chunk);
 static unsigned int dma_bitsize;
 integer_param("dma_bits", dma_bitsize);
 
-#define round_pgdown(_p)  ((_p)&PAGE_MASK)
-#define round_pgup(_p)(((_p)+(PAGE_SIZE-1))&PAGE_MASK)
-
 /* Offlined page list, protected by heap_lock. */
 PAGE_LIST_HEAD(page_offlined_list);
 /* Broken page list, protected by heap_lock. */
diff --git a/xen/include/xen/pfn.h b/xen/include/xen/pfn.h
index 3626197de3..ed9d27d568 100644
--- a/xen/include/xen/pfn.h
+++ b/xen/include/xen/pfn.h
@@ -6,4 +6,7 @@
 #define PFN_DOWN(x)   ((x) >> PAGE_SHIFT)
 #define PFN_UP(x) (((x) + PAGE_SIZE-1) >> PAGE_SHIFT)
 
+#define round_pgup(_p)(((_p)+(PAGE_SIZE-1))&PAGE_MASK)
+#define round_pgdown(_p)  ((_p)&PAGE_MASK)
+
 #endif /* __XEN_PFN_H__ */
-- 
2.11.0


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

[Xen-devel] [PATCH RFC 0/5] Refactor x86 domain builder

[Wei Liu]

Long term we want to separate different sets of guest supporting code. We would
like to have CONFIG_HVM and CONFIG_PV (maybe even CONFIG_PVH?).  I will start
to dientangle Xen code component by component. This is also somewhat related to
the project to move PV interface inside a PVH container, which I plan to work
on soon.

RFC because I would like to know whether I should start putting them into pv
and hvm directory respectively. And I would also like to know the reception of
the idea to disentangle Xen guest supporting code in general.

This series is built on top of Roger's PVHv1 removal series.

Wei Liu (5):
  xen: move round_pg{up,down} to pfn.h
  xen: include xen/types.h in domain.h
  x86: split PV dom0 builder to domain_build_pv.c
  x86: split PVH dom0 builder to domain_build_pv.c
  x86: clean up header files in domain_build.c

 xen/arch/x86/Makefile   |2 +
 xen/arch/x86/domain_build.c | 2003 +--
 xen/arch/x86/domain_build.h |   40 +
 xen/arch/x86/domain_build_pv.c  |  909 ++
 xen/arch/x86/domain_build_pvh.c | 1097 +
 xen/common/page_alloc.c |4 +-
 xen/include/xen/domain.h|2 +
 xen/include/xen/pfn.h   |3 +
 8 files changed, 2070 insertions(+), 1990 deletions(-)
 create mode 100644 xen/arch/x86/domain_build.h
 create mode 100644 xen/arch/x86/domain_build_pv.c
 create mode 100644 xen/arch/x86/domain_build_pvh.c

-- 
2.11.0


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

Re: [Xen-devel] [PATCH 2/6] xen: credit: (micro) optimize csched_runq_steal().

[anshul makkar]

On 02/03/17 10:38, Dario Faggioli wrote:

Chacking whether or not a vCPU can be 'stolen'
from a peer pCPU's runqueue is relatively cheap.

Therefore, let's do that  as early as possible,
avoiding potentially useless complex checks, and
cpumask manipulations.

Signed-off-by: Dario Faggioli 
---
Cc: George Dunlap 
---
  xen/common/sched_credit.c |   17 +
  1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 63a8675..2b13e99 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -708,12 +708,10 @@ static inline int
  __csched_vcpu_is_migrateable(struct vcpu *vc, int dest_cpu, cpumask_t *mask)
  {
  /*
- * Don't pick up work that's in the peer's scheduling tail or hot on
- * peer PCPU. Only pick up work that prefers and/or is allowed to run
- * on our CPU.
+ * Don't pick up work that's or hot on peer PCPU, or that can't (or

Not clear.

+ * would prefer not to) run on cpu.
   */
-return !vc->is_running &&
-   !__csched_vcpu_is_cache_hot(vc) &&
+return !__csched_vcpu_is_cache_hot(vc) &&
 cpumask_test_cpu(dest_cpu, mask);
!vc->is_running doesn't ease the complexity and doesn't save much on cpu 
cycles. Infact, I think (!vc->is_running) makes the intention to check 
for migration much more clear to understand.


Yeah, apart from the above reasons, its save to remove this check from here.

  }
  
@@ -1622,7 +1620,9 @@ csched_runq_steal(int peer_cpu, int cpu, int pri, int balance_step)

  BUG_ON( is_idle_vcpu(vc) );
  
  /*

- * If the vcpu has no useful soft affinity, skip this vcpu.
+ * If the vcpu is still in peer_cpu's scheduling tail, or if it
+ * has no useful soft affinity, skip it.
+ *
   * In fact, what we want is to check if we have any "soft-affine
   * work" to steal, before starting to look at "hard-affine work".
   *
@@ -1633,8 +1633,9 @@ csched_runq_steal(int peer_cpu, int cpu, int pri, int 
balance_step)
   * vCPUs with useful soft affinities in some sort of bitmap
   * or counter.
   */
-if ( balance_step == CSCHED_BALANCE_SOFT_AFFINITY
- && !__vcpu_has_soft_affinity(vc, vc->cpu_hard_affinity) )
+if ( vc->is_running ||
+ (balance_step == CSCHED_BALANCE_SOFT_AFFINITY
+  && !__vcpu_has_soft_affinity(vc, vc->cpu_hard_affinity)) )
  continue;
  
  csched_balance_cpumask(vc, balance_step, cpumask_scratch);



___
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 v2 5/5] xen: use libxendevicemodel when available

[Paul Durrant]

> -Original Message-
> From: Stefano Stabellini [mailto:sstabell...@kernel.org]
> Sent: 02 March 2017 22:50
> To: Paul Durrant 
> Cc: xen-de...@lists.xenproject.org; qemu-de...@nongnu.org; Stefano
> Stabellini ; Anthony Perard
> 
> Subject: Re: [PATCH v2 5/5] xen: use libxendevicemodel when available
> 
> On Thu, 2 Mar 2017, Paul Durrant wrote:
> > This patch modifies the wrapper functions in xen_common.h to use the
> > new xendevicemodel interface if it is available along with compatibility
> > code to use the old libxenctrl interface if it is not.
> >
> > Signed-off-by: Paul Durrant 
> > ---
> > Cc: Stefano Stabellini 
> > Cc: Anthony Perard 
> >
> > v2:
> > - Add a compat define for xenforeignmemory_close() since this is now
> >   used.
> > ---
> >  include/hw/xen/xen_common.h | 115
> +++-
> >  xen-common.c|   8 +++
> >  2 files changed, 90 insertions(+), 33 deletions(-)
> >
> > diff --git a/include/hw/xen/xen_common.h
> b/include/hw/xen/xen_common.h
> > index 31cf25f..48444e5 100644
> > --- a/include/hw/xen/xen_common.h
> > +++ b/include/hw/xen/xen_common.h
> > @@ -9,6 +9,7 @@
> >  #undef XC_WANT_COMPAT_EVTCHN_API
> >  #undef XC_WANT_COMPAT_GNTTAB_API
> >  #undef XC_WANT_COMPAT_MAP_FOREIGN_API
> > +#undef XC_WANT_COMPAT_DEVICEMODEL_API
> >
> >  #include 
> >  #include 
> > @@ -26,48 +27,95 @@ extern xc_interface *xen_xc;
> >   * We don't support Xen prior to 4.2.0.
> >   */
> >
> > +#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 490
> > +
> > +typedef xc_interface xendevicemodel_handle;
> > +
> > +#define xendevicemodel_open(l, f) xen_xc
> > +
> > +#define xendevicemodel_map_io_range_to_ioreq_server \
> > +xc_hvm_map_io_range_to_ioreq_server
> > +#define xendevicemodel_unmap_io_range_from_ioreq_server \
> > +xc_hvm_unmap_io_range_from_ioreq_server
> > +#define xendevicemodel_map_pcidev_to_ioreq_server \
> > +xc_hvm_map_pcidev_to_ioreq_server
> > +#define xendevicemodel_unmap_pcidev_from_ioreq_server \
> > +xc_hvm_unmap_pcidev_from_ioreq_server
> > +#define xendevicemodel_create_ioreq_server \
> > +xc_hvm_create_ioreq_server
> > +#define xendevicemodel_destroy_ioreq_server \
> > +xc_hvm_destroy_ioreq_server
> > +#define xendevicemodel_get_ioreq_server_info \
> > +xc_hvm_get_ioreq_server_info
> > +#define xendevicemodel_set_ioreq_server_state \
> > +xc_hvm_set_ioreq_server_state
> > +#define xendevicemodel_set_pci_intx_level \
> > +xc_hvm_set_pci_intx_level
> > +#define xendevicemodel_set_pci_link_route \
> > +xc_hvm_set_pci_link_route
> > +#define xendevicemodel_set_isa_irq_level \
> > +xc_hvm_set_isa_irq_level
> > +#define xendevicemodel_inject_msi \
> > +xc_hvm_inject_msi
> > +#define xendevicemodel_set_mem_type \
> > +xc_hvm_set_mem_type
> > +#define xendevicemodel_track_dirty_vram \
> > +xc_hvm_track_dirty_vram
> > +#define xendevicemodel_modified_memory \
> > +xc_hvm_modified_memory
> 
> It does build correctly now for Xen < 4.9, however it breaks against
> xen-unstable:

That's weird. I built it against staging just before I posted it. I must have 
something different in my environment. I'll check again.

> 
>   ERROR: configure test passed without -Werror but failed with -Werror.
>  This is probably a bug in the configure script. The failing command
>  will be at the bottom of config.log.
>  You can run configure with --disable-werror to bypass this check.
> 
> and config.log says:
> 
>   config-temp/qemu-conf.c: In function 'main':
>   config-temp/qemu-conf.c:32:3: error: implicit declaration of function
> 'xc_hvm_set_mem_type' [-Werror=implicit-function-declaration]
>   config-temp/qemu-conf.c:32:3: error: nested extern declaration of
> 'xc_hvm_set_mem_type' [-Werror=nested-externs]
>   config-temp/qemu-conf.c:34:3: error: implicit declaration of function
> 'xc_hvm_inject_msi' [-Werror=implicit-function-declaration]
>   config-temp/qemu-conf.c:34:3: error: nested extern declaration of
> 'xc_hvm_inject_msi' [-Werror=nested-externs]
>   config-temp/qemu-conf.c:35:3: error: implicit declaration of function
> 'xc_hvm_create_ioreq_server' [-Werror=implicit-function-declaration]
>   config-temp/qemu-conf.c:35:3: error: nested extern declaration of
> 'xc_hvm_create_ioreq_server' [-Werror=nested-externs]
> 
> 
> With -DXC_WANT_COMPAT_DEVICEMODEL_API=1:
> 
>   In file included from /local/qemu-
> upstream/include/hw/xen/xen_backend.h:4:0,
>from hw/block/xen_disk.c:27:
>   /local/qemu-upstream/include/hw/xen/xen_common.h: In function
> 'xen_set_mem_type':
>   /local/qemu-upstream/include/hw/xen/xen_common.h:78:5: error: implicit
> declaration of function 'xc_hvm_set_mem_type' [-Werror=implicit-function-
> declaration]
> 
> 
> Only another comment (I guess you didn't see in my previous email):
> could you please use static inline functions for compatibility rather
> than macros? That way we are going to have some more compile time
> checks. Or at 

Re: [Xen-devel] [PATCH] x86/cpuid: Fix booting on AMD Phenom 6-core platform

[Jan Beulich]

>>> On 02.03.17 at 21:02,  wrote:
> c/s 5cecf60f4 "x86/cpuid: Handle leaf 0x1 in guest_cpuid()" causes Linux 4.10
> to crash during boot.
> 
> It turns out to be because of the reported apic_id, which was altered to be
> more consistent across guests.  Revert back to the previous behaviour, by
> limiting the apic_id adjustment to HVM guests only.  Whomever gets to fixes
> topology representation is going to have a lot of fun with non-power-of-2 AMD
> boxes.
> 
> Reported-by: Sander Eikelenboom 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Jan Beulich 



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

Re: [Xen-devel] [PATCH RFC 0/5] Refactor x86 domain builder

[Jan Beulich]

>>> On 03.03.17 at 10:41,  wrote:
> Long term we want to separate different sets of guest supporting code. We 
> would
> like to have CONFIG_HVM and CONFIG_PV (maybe even CONFIG_PVH?).  I will start
> to dientangle Xen code component by component. This is also somewhat related 
> to
> the project to move PV interface inside a PVH container, which I plan to work
> on soon.
> 
> RFC because I would like to know whether I should start putting them into pv
> and hvm directory respectively.

Yes please, along the lines of an RFC patch Andrew had sent not sao
long ago.

> And I would also like to know the reception of
> the idea to disentangle Xen guest supporting code in general.

I think this is generally a good idea, provided it doesn't end up
requiring too many clumsy internal interfaces (e.g. when trying to
share code).

Jan


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

Re: [Xen-devel] [PATCH RFC 1/5] xen: move round_pg{up, down} to pfn.h

[Jan Beulich]

>>> On 03.03.17 at 10:41,  wrote:
> --- a/xen/include/xen/pfn.h
> +++ b/xen/include/xen/pfn.h
> @@ -6,4 +6,7 @@
>  #define PFN_DOWN(x)   ((x) >> PAGE_SHIFT)
>  #define PFN_UP(x) (((x) + PAGE_SIZE-1) >> PAGE_SHIFT)
>  
> +#define round_pgup(_p)(((_p)+(PAGE_SIZE-1))&PAGE_MASK)
> +#define round_pgdown(_p)  ((_p)&PAGE_MASK)

With blanks around binary operators added and leading underscores
removed I think this could go in right away, i.e.
Reviewed-by: Jan Beulich 

Jan


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

Re: [Xen-devel] [PATCH RFC 2/5] xen: include xen/types.h in domain.h

[Jan Beulich]

>>> On 03.03.17 at 10:41,  wrote:
> The public header expects a few types to be present.
> 
> This works in the code base only because types.h is included by some
> other headers which happen to be placed before the inclusion of
> domain.h.
> 
> Include types.h before xen.h in domain.h to fix it properly.
> 
> Signed-off-by: Wei Liu 

Acked-by: Jan Beulich 



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

Re: [Xen-devel] [PATCH 1/2] x86/hvm: Don't raise #GP behind the emulators back for CR accesses

[Jan Beulich]

>>> On 02.03.17 at 15:59,  wrote:
> hvm_set_cr{0,4}() are reachable from the emulator, but use
> hvm_inject_hw_exception() directly.
> 
> Alter the API to make the callers of hvm_set_cr{0,3,4}() responsible for
> raising #GP, and apply this change to all existing callers.

As you're touching CR-write paths only, would you mind changing
the title to say "writes" instead of "accesses"?

> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -1046,9 +1046,18 @@ static void load_shadow_guest_state(struct vcpu *v)
>  
>  nvcpu->guest_cr[0] = get_vvmcs(v, CR0_READ_SHADOW);
>  nvcpu->guest_cr[4] = get_vvmcs(v, CR4_READ_SHADOW);
> -hvm_set_cr0(get_vvmcs(v, GUEST_CR0), 1);
> -hvm_set_cr4(get_vvmcs(v, GUEST_CR4), 1);
> -hvm_set_cr3(get_vvmcs(v, GUEST_CR3), 1);
> +
> +rc = hvm_set_cr0(get_vvmcs(v, GUEST_CR0), 1);
> +if ( rc == X86EMUL_EXCEPTION )
> +hvm_inject_hw_exception(TRAP_gp_fault, 0);
> +
> +rc = hvm_set_cr4(get_vvmcs(v, GUEST_CR4), 1);
> +if ( rc == X86EMUL_EXCEPTION )
> +hvm_inject_hw_exception(TRAP_gp_fault, 0);
> +
> +rc = hvm_set_cr3(get_vvmcs(v, GUEST_CR3), 1);
> +if ( rc == X86EMUL_EXCEPTION )
> +hvm_inject_hw_exception(TRAP_gp_fault, 0);

While indeed not a change in behavior, this multiple raising of #GP
is so wrong that I wonder whether it shouldn't be fixed while you're
touching it: Simply accumulate the need to raise #GP, and do so
once at the end.

> @@ -1237,9 +1246,17 @@ static void load_vvmcs_host_state(struct vcpu *v)
>  __vmwrite(vmcs_h2g_field[i].guest_field, r);
>  }
>  
> -hvm_set_cr0(get_vvmcs(v, HOST_CR0), 1);
> -hvm_set_cr4(get_vvmcs(v, HOST_CR4), 1);
> -hvm_set_cr3(get_vvmcs(v, HOST_CR3), 1);
> +rc = hvm_set_cr0(get_vvmcs(v, HOST_CR0), 1);
> +if ( rc == X86EMUL_EXCEPTION )
> +hvm_inject_hw_exception(TRAP_gp_fault, 0);
> +
> +rc = hvm_set_cr4(get_vvmcs(v, HOST_CR4), 1);
> +if ( rc == X86EMUL_EXCEPTION )
> +hvm_inject_hw_exception(TRAP_gp_fault, 0);
> +
> +rc = hvm_set_cr3(get_vvmcs(v, HOST_CR3), 1);
> +if ( rc == X86EMUL_EXCEPTION )
> +hvm_inject_hw_exception(TRAP_gp_fault, 0);

Same here then obviously.

Either way
Reviewed-by: Jan Beulich 

Jan


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

Re: [Xen-devel] [PATCH RFC 1/5] xen: move round_pg{up, down} to pfn.h

[Wei Liu]

On Fri, Mar 03, 2017 at 03:05:05AM -0700, Jan Beulich wrote:
> >>> On 03.03.17 at 10:41,  wrote:
> > --- a/xen/include/xen/pfn.h
> > +++ b/xen/include/xen/pfn.h
> > @@ -6,4 +6,7 @@
> >  #define PFN_DOWN(x)   ((x) >> PAGE_SHIFT)
> >  #define PFN_UP(x) (((x) + PAGE_SIZE-1) >> PAGE_SHIFT)
> >  
> > +#define round_pgup(_p)(((_p)+(PAGE_SIZE-1))&PAGE_MASK)
> > +#define round_pgdown(_p)  ((_p)&PAGE_MASK)
> 
> With blanks around binary operators added and leading underscores
> removed I think this could go in right away, i.e.
> Reviewed-by: Jan Beulich 
> 

Right. Thanks. I will commit the following hunk with your reviewed-by.

#define round_pgup(p)(((p) + (PAGE_SIZE - 1)) & PAGE_MASK)
#define round_pgdown(p)  ((p) & PAGE_MASK)

> Jan
> 

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

Re: [Xen-devel] [PATCH 2/2] x86/emul: Hold x86_emulate() to strict X86EMUL_EXCEPTION requirements

[Jan Beulich]

>>> On 02.03.17 at 15:59,  wrote:
> All known paths raising faults behind the back of the emulator have fixed.

"have been fixed"?

> Reinstate the original intended assertion concerning the behaviour of
> X86EMUL_EXCEPTION and ctxt->event_pending.
> 
> As x86_emulate_wrapper() now covers both PV and HVM guests properly, there is
> no need for the PV assertions following calls to x86_emulate().
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Jan Beulich 



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

Re: [Xen-devel] [PATCH 1/2] x86/hvm: Don't raise #GP behind the emulators back for CR accesses

[Andrew Cooper]

On 03/03/17 10:16, Jan Beulich wrote:
 On 02.03.17 at 15:59,  wrote:
>> hvm_set_cr{0,4}() are reachable from the emulator, but use
>> hvm_inject_hw_exception() directly.
>>
>> Alter the API to make the callers of hvm_set_cr{0,3,4}() responsible for
>> raising #GP, and apply this change to all existing callers.
> As you're touching CR-write paths only, would you mind changing
> the title to say "writes" instead of "accesses"?

Ok.

>
>> --- a/xen/arch/x86/hvm/vmx/vvmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
>> @@ -1046,9 +1046,18 @@ static void load_shadow_guest_state(struct vcpu *v)
>>  
>>  nvcpu->guest_cr[0] = get_vvmcs(v, CR0_READ_SHADOW);
>>  nvcpu->guest_cr[4] = get_vvmcs(v, CR4_READ_SHADOW);
>> -hvm_set_cr0(get_vvmcs(v, GUEST_CR0), 1);
>> -hvm_set_cr4(get_vvmcs(v, GUEST_CR4), 1);
>> -hvm_set_cr3(get_vvmcs(v, GUEST_CR3), 1);
>> +
>> +rc = hvm_set_cr0(get_vvmcs(v, GUEST_CR0), 1);
>> +if ( rc == X86EMUL_EXCEPTION )
>> +hvm_inject_hw_exception(TRAP_gp_fault, 0);
>> +
>> +rc = hvm_set_cr4(get_vvmcs(v, GUEST_CR4), 1);
>> +if ( rc == X86EMUL_EXCEPTION )
>> +hvm_inject_hw_exception(TRAP_gp_fault, 0);
>> +
>> +rc = hvm_set_cr3(get_vvmcs(v, GUEST_CR3), 1);
>> +if ( rc == X86EMUL_EXCEPTION )
>> +hvm_inject_hw_exception(TRAP_gp_fault, 0);
> While indeed not a change in behavior, this multiple raising of #GP
> is so wrong that I wonder whether it shouldn't be fixed while you're
> touching it: Simply accumulate the need to raise #GP, and do so
> once at the end.
>
>> @@ -1237,9 +1246,17 @@ static void load_vvmcs_host_state(struct vcpu *v)
>>  __vmwrite(vmcs_h2g_field[i].guest_field, r);
>>  }
>>  
>> -hvm_set_cr0(get_vvmcs(v, HOST_CR0), 1);
>> -hvm_set_cr4(get_vvmcs(v, HOST_CR4), 1);
>> -hvm_set_cr3(get_vvmcs(v, HOST_CR3), 1);
>> +rc = hvm_set_cr0(get_vvmcs(v, HOST_CR0), 1);
>> +if ( rc == X86EMUL_EXCEPTION )
>> +hvm_inject_hw_exception(TRAP_gp_fault, 0);
>> +
>> +rc = hvm_set_cr4(get_vvmcs(v, HOST_CR4), 1);
>> +if ( rc == X86EMUL_EXCEPTION )
>> +hvm_inject_hw_exception(TRAP_gp_fault, 0);
>> +
>> +rc = hvm_set_cr3(get_vvmcs(v, HOST_CR3), 1);
>> +if ( rc == X86EMUL_EXCEPTION )
>> +hvm_inject_hw_exception(TRAP_gp_fault, 0);
> Same here then obviously.

In both cases, raising #GP at all is wrong.  All values should have been
properly audited at vmwrite time, so a failure here should probably be
domain_crash().

>
> Either way
> Reviewed-by: Jan Beulich 

Ideally I'd prefer not to mix multiple functional changes into a single
patch.

~Andrew

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

Re: [Xen-devel] [PATCH 1/2] x86/hvm: Don't raise #GP behind the emulators back for CR accesses

[Jan Beulich]

>>> On 03.03.17 at 11:30,  wrote:
> On 03/03/17 10:16, Jan Beulich wrote:
> On 02.03.17 at 15:59,  wrote:
>>> --- a/xen/arch/x86/hvm/vmx/vvmx.c
>>> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
>>> @@ -1046,9 +1046,18 @@ static void load_shadow_guest_state(struct vcpu *v)
>>>  
>>>  nvcpu->guest_cr[0] = get_vvmcs(v, CR0_READ_SHADOW);
>>>  nvcpu->guest_cr[4] = get_vvmcs(v, CR4_READ_SHADOW);
>>> -hvm_set_cr0(get_vvmcs(v, GUEST_CR0), 1);
>>> -hvm_set_cr4(get_vvmcs(v, GUEST_CR4), 1);
>>> -hvm_set_cr3(get_vvmcs(v, GUEST_CR3), 1);
>>> +
>>> +rc = hvm_set_cr0(get_vvmcs(v, GUEST_CR0), 1);
>>> +if ( rc == X86EMUL_EXCEPTION )
>>> +hvm_inject_hw_exception(TRAP_gp_fault, 0);
>>> +
>>> +rc = hvm_set_cr4(get_vvmcs(v, GUEST_CR4), 1);
>>> +if ( rc == X86EMUL_EXCEPTION )
>>> +hvm_inject_hw_exception(TRAP_gp_fault, 0);
>>> +
>>> +rc = hvm_set_cr3(get_vvmcs(v, GUEST_CR3), 1);
>>> +if ( rc == X86EMUL_EXCEPTION )
>>> +hvm_inject_hw_exception(TRAP_gp_fault, 0);
>> While indeed not a change in behavior, this multiple raising of #GP
>> is so wrong that I wonder whether it shouldn't be fixed while you're
>> touching it: Simply accumulate the need to raise #GP, and do so
>> once at the end.
>>
>>> @@ -1237,9 +1246,17 @@ static void load_vvmcs_host_state(struct vcpu *v)
>>>  __vmwrite(vmcs_h2g_field[i].guest_field, r);
>>>  }
>>>  
>>> -hvm_set_cr0(get_vvmcs(v, HOST_CR0), 1);
>>> -hvm_set_cr4(get_vvmcs(v, HOST_CR4), 1);
>>> -hvm_set_cr3(get_vvmcs(v, HOST_CR3), 1);
>>> +rc = hvm_set_cr0(get_vvmcs(v, HOST_CR0), 1);
>>> +if ( rc == X86EMUL_EXCEPTION )
>>> +hvm_inject_hw_exception(TRAP_gp_fault, 0);
>>> +
>>> +rc = hvm_set_cr4(get_vvmcs(v, HOST_CR4), 1);
>>> +if ( rc == X86EMUL_EXCEPTION )
>>> +hvm_inject_hw_exception(TRAP_gp_fault, 0);
>>> +
>>> +rc = hvm_set_cr3(get_vvmcs(v, HOST_CR3), 1);
>>> +if ( rc == X86EMUL_EXCEPTION )
>>> +hvm_inject_hw_exception(TRAP_gp_fault, 0);
>> Same here then obviously.
> 
> In both cases, raising #GP at all is wrong.

Of course.

>  All values should have been
> properly audited at vmwrite time, so a failure here should probably be
> domain_crash().

That would be better than #DF.

>> Either way
>> Reviewed-by: Jan Beulich 
> 
> Ideally I'd prefer not to mix multiple functional changes into a single
> patch.

As said - either way.

Jan


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

Re: [Xen-devel] [PATCH v2 5/5] xen: use libxendevicemodel when available

[Paul Durrant]

> -Original Message-
> From: Stefano Stabellini [mailto:sstabell...@kernel.org]
> Sent: 02 March 2017 22:50
> To: Paul Durrant 
> Cc: xen-de...@lists.xenproject.org; qemu-de...@nongnu.org; Stefano
> Stabellini ; Anthony Perard
> 
> Subject: Re: [PATCH v2 5/5] xen: use libxendevicemodel when available
> 
> On Thu, 2 Mar 2017, Paul Durrant wrote:
> > This patch modifies the wrapper functions in xen_common.h to use the
> > new xendevicemodel interface if it is available along with compatibility
> > code to use the old libxenctrl interface if it is not.
> >
> > Signed-off-by: Paul Durrant 
> > ---
> > Cc: Stefano Stabellini 
> > Cc: Anthony Perard 
> >
> > v2:
> > - Add a compat define for xenforeignmemory_close() since this is now
> >   used.
> > ---
> >  include/hw/xen/xen_common.h | 115
> +++-
> >  xen-common.c|   8 +++
> >  2 files changed, 90 insertions(+), 33 deletions(-)
> >
> > diff --git a/include/hw/xen/xen_common.h
> b/include/hw/xen/xen_common.h
> > index 31cf25f..48444e5 100644
> > --- a/include/hw/xen/xen_common.h
> > +++ b/include/hw/xen/xen_common.h
> > @@ -9,6 +9,7 @@
> >  #undef XC_WANT_COMPAT_EVTCHN_API
> >  #undef XC_WANT_COMPAT_GNTTAB_API
> >  #undef XC_WANT_COMPAT_MAP_FOREIGN_API
> > +#undef XC_WANT_COMPAT_DEVICEMODEL_API
> >
> >  #include 
> >  #include 
> > @@ -26,48 +27,95 @@ extern xc_interface *xen_xc;
> >   * We don't support Xen prior to 4.2.0.
> >   */
> >
> > +#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 490
> > +
> > +typedef xc_interface xendevicemodel_handle;
> > +
> > +#define xendevicemodel_open(l, f) xen_xc
> > +
> > +#define xendevicemodel_map_io_range_to_ioreq_server \
> > +xc_hvm_map_io_range_to_ioreq_server
> > +#define xendevicemodel_unmap_io_range_from_ioreq_server \
> > +xc_hvm_unmap_io_range_from_ioreq_server
> > +#define xendevicemodel_map_pcidev_to_ioreq_server \
> > +xc_hvm_map_pcidev_to_ioreq_server
> > +#define xendevicemodel_unmap_pcidev_from_ioreq_server \
> > +xc_hvm_unmap_pcidev_from_ioreq_server
> > +#define xendevicemodel_create_ioreq_server \
> > +xc_hvm_create_ioreq_server
> > +#define xendevicemodel_destroy_ioreq_server \
> > +xc_hvm_destroy_ioreq_server
> > +#define xendevicemodel_get_ioreq_server_info \
> > +xc_hvm_get_ioreq_server_info
> > +#define xendevicemodel_set_ioreq_server_state \
> > +xc_hvm_set_ioreq_server_state
> > +#define xendevicemodel_set_pci_intx_level \
> > +xc_hvm_set_pci_intx_level
> > +#define xendevicemodel_set_pci_link_route \
> > +xc_hvm_set_pci_link_route
> > +#define xendevicemodel_set_isa_irq_level \
> > +xc_hvm_set_isa_irq_level
> > +#define xendevicemodel_inject_msi \
> > +xc_hvm_inject_msi
> > +#define xendevicemodel_set_mem_type \
> > +xc_hvm_set_mem_type
> > +#define xendevicemodel_track_dirty_vram \
> > +xc_hvm_track_dirty_vram
> > +#define xendevicemodel_modified_memory \
> > +xc_hvm_modified_memory
> 
> It does build correctly now for Xen < 4.9, however it breaks against
> xen-unstable:
> 
>   ERROR: configure test passed without -Werror but failed with -Werror.
>  This is probably a bug in the configure script. The failing command
>  will be at the bottom of config.log.
>  You can run configure with --disable-werror to bypass this check.
> 
> and config.log says:
> 
>   config-temp/qemu-conf.c: In function 'main':
>   config-temp/qemu-conf.c:32:3: error: implicit declaration of function
> 'xc_hvm_set_mem_type' [-Werror=implicit-function-declaration]
>   config-temp/qemu-conf.c:32:3: error: nested extern declaration of
> 'xc_hvm_set_mem_type' [-Werror=nested-externs]
>   config-temp/qemu-conf.c:34:3: error: implicit declaration of function
> 'xc_hvm_inject_msi' [-Werror=implicit-function-declaration]
>   config-temp/qemu-conf.c:34:3: error: nested extern declaration of
> 'xc_hvm_inject_msi' [-Werror=nested-externs]
>   config-temp/qemu-conf.c:35:3: error: implicit declaration of function
> 'xc_hvm_create_ioreq_server' [-Werror=implicit-function-declaration]
>   config-temp/qemu-conf.c:35:3: error: nested extern declaration of
> 'xc_hvm_create_ioreq_server' [-Werror=nested-externs]
> 
> 
> With -DXC_WANT_COMPAT_DEVICEMODEL_API=1:
> 
>   In file included from /local/qemu-
> upstream/include/hw/xen/xen_backend.h:4:0,
>from hw/block/xen_disk.c:27:
>   /local/qemu-upstream/include/hw/xen/xen_common.h: In function
> 'xen_set_mem_type':
>   /local/qemu-upstream/include/hw/xen/xen_common.h:78:5: error: implicit
> declaration of function 'xc_hvm_set_mem_type' [-Werror=implicit-function-
> declaration]
> 

Ah... Do you have Anthony's patch?

http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=9970e98ace48574701f7e2286fb67090481a3fec

I suspect that may be the problem you're seeing above.

  Paul

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

Re: [Xen-devel] [PATCH] x86/cpuid: Fix booting on AMD Phenom 6-core platform

[Andrew Cooper]

On 02/03/17 20:02, Andrew Cooper wrote:
> c/s 5cecf60f4 "x86/cpuid: Handle leaf 0x1 in guest_cpuid()" causes Linux 4.10
> to crash during boot.
>
> It turns out to be because of the reported apic_id, which was altered to be
> more consistent across guests.  Revert back to the previous behaviour, by
> limiting the apic_id adjustment to HVM guests only.  Whomever gets to fixes
> topology representation is going to have a lot of fun with non-power-of-2 AMD
> boxes.
>
> Reported-by: Sander Eikelenboom 
> Signed-off-by: Andrew Cooper 

Sorry I forgot to CC you this.  (I keep forgetting that git doesn't
understand Reported-by when collecting its CC list.)

Would you mind double checking this patch please?

~Andrew

> ---
> CC: Jan Beulich 
> ---
>  xen/arch/x86/cpuid.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
> index 0dd35dc..d6f6b88 100644
> --- a/xen/arch/x86/cpuid.c
> +++ b/xen/arch/x86/cpuid.c
> @@ -749,7 +749,8 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
>  case 0x1:
>  /* TODO: Rework topology logic. */
>  res->b &= 0x00ffu;
> -res->b |= (v->vcpu_id * 2) << 24;
> +if ( has_hvm_container_domain(d) )
> +res->b |= (v->vcpu_id * 2) << 24;
>  
>  /* TODO: Rework vPMU control in terms of toolstack choices. */
>  if ( vpmu_available(v) &&


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

Re: [Xen-devel] [PATCH] x86/cpuid: Fix booting on AMD Phenom 6-core platform

[Sander Eikelenboom]

On 03/03/17 11:40, Andrew Cooper wrote:
> On 02/03/17 20:02, Andrew Cooper wrote:
>> c/s 5cecf60f4 "x86/cpuid: Handle leaf 0x1 in guest_cpuid()" causes Linux 4.10
>> to crash during boot.
>>
>> It turns out to be because of the reported apic_id, which was altered to be
>> more consistent across guests.  Revert back to the previous behaviour, by
>> limiting the apic_id adjustment to HVM guests only.  Whomever gets to fixes
>> topology representation is going to have a lot of fun with non-power-of-2 AMD
>> boxes.
>>
>> Reported-by: Sander Eikelenboom 
>> Signed-off-by: Andrew Cooper 
> 
> Sorry I forgot to CC you this.  (I keep forgetting that git doesn't
> understand Reported-by when collecting its CC list.)
> 
> Would you mind double checking this patch please?

Sure (but it will take till somewhere this evening CET though).

--
Sander

> ~Andrew
> 
>> ---
>> CC: Jan Beulich 
>> ---
>>  xen/arch/x86/cpuid.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
>> index 0dd35dc..d6f6b88 100644
>> --- a/xen/arch/x86/cpuid.c
>> +++ b/xen/arch/x86/cpuid.c
>> @@ -749,7 +749,8 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
>>  case 0x1:
>>  /* TODO: Rework topology logic. */
>>  res->b &= 0x00ffu;
>> -res->b |= (v->vcpu_id * 2) << 24;
>> +if ( has_hvm_container_domain(d) )
>> +res->b |= (v->vcpu_id * 2) << 24;
>>  
>>  /* TODO: Rework vPMU control in terms of toolstack choices. */
>>  if ( vpmu_available(v) &&
> 


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

Re: [Xen-devel] [PATCH] x86/cpuid: Fix booting on AMD Phenom 6-core platform

[Andrew Cooper]

On 03/03/17 10:47, Sander Eikelenboom wrote:
> On 03/03/17 11:40, Andrew Cooper wrote:
>> On 02/03/17 20:02, Andrew Cooper wrote:
>>> c/s 5cecf60f4 "x86/cpuid: Handle leaf 0x1 in guest_cpuid()" causes Linux 
>>> 4.10
>>> to crash during boot.
>>>
>>> It turns out to be because of the reported apic_id, which was altered to be
>>> more consistent across guests.  Revert back to the previous behaviour, by
>>> limiting the apic_id adjustment to HVM guests only.  Whomever gets to fixes
>>> topology representation is going to have a lot of fun with non-power-of-2 
>>> AMD
>>> boxes.
>>>
>>> Reported-by: Sander Eikelenboom 
>>> Signed-off-by: Andrew Cooper 
>> Sorry I forgot to CC you this.  (I keep forgetting that git doesn't
>> understand Reported-by when collecting its CC list.)
>>
>> Would you mind double checking this patch please?
> Sure (but it will take till somewhere this evening CET though).

No problem.  Thanks.

~Andrew

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

Re: [Xen-devel] [PATCH v9 1/8] VMX: Permanently assign PI hook vmx_pi_switch_to()

[Jan Beulich]

>>> On 03.03.17 at 09:29,  wrote:
>>  From: Jan Beulich [mailto:jbeul...@suse.com]
>> Sent: Wednesday, March 01, 2017 3:42 PM
>> Sounds good to me (read: Reviewed-by: Jan Beulich ).
>> If Kevin would give his ack, I could replace the comment while committing,
>> so you wouldn't need to re-send.
> 
> Good to me too.
> 
> Acked-by: Kevin Tian 

How about patch 3 of this series then? Patch 4 already has your ack,
and patches 5 and onwards need another iteration afaict.

Jan


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

Re: [Xen-devel] [PATCH v1 2/2] x86/vvmx: add vmcs id check into vmptrld emulation

[Jan Beulich]

>>> On 01.03.17 at 12:01,  wrote:
> On 01/03/17 09:13, Sergey Dyasli wrote:
>> --- a/xen/arch/x86/hvm/vmx/vvmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
>> @@ -1633,6 +1633,17 @@ int nvmx_handle_vmptrld(struct cpu_user_regs *regs)
>>  {
>>  if ( writable )
>>  {
>> +struct vmcs_struct *vvmcs = vvmcx;
>> +
>> +if ( ((vvmcs->vmcs_revision_id ^ vmx_basic_msr) &
>> + VMX_BASIC_REVISION_MASK) ||
>> + (!cpu_has_vmx_vmcs_shadowing &&
>> +  (vvmcs->vmcs_revision_id & ~VMX_BASIC_REVISION_MASK)) 
>> )
>> +{
>> +hvm_unmap_guest_frame(vvmcx, 1);
>> +vmfail(regs, VMX_INSN_VMPTRLD_INCORRECT_VMCS_ID);
> 
> A newline here please (can be fixed on commit if there are no other
> issues).  Otherwise, Reviewed-by: Andrew Cooper 
> 
>> +return X86EMUL_OKAY;
>> +}

I've added one, but commonly we require such only on the main
(final) return statement of a function.

Jan


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

Re: [Xen-devel] [PATCH RFC 0/5] Refactor x86 domain builder

[Andrew Cooper]

On 03/03/17 09:41, Wei Liu wrote:
> Long term we want to separate different sets of guest supporting code. We 
> would
> like to have CONFIG_HVM and CONFIG_PV (maybe even CONFIG_PVH?).

Probably not PVH.  The only differences between HVM and PVH from Xen's
point of view are some of the emulation choices.

> I will start
> to dientangle Xen code component by component. This is also somewhat related 
> to
> the project to move PV interface inside a PVH container, which I plan to work
> on soon.
>
> RFC because I would like to know whether I should start putting them into pv
> and hvm directory respectively. And I would also like to know the reception of
> the idea to disentangle Xen guest supporting code in general.

+10, although I have a couple of further suggestions.

If we are going to start this, I think we should actually include
CONFIG_{PV,HVM} so we can make the files compilation-safe as they are
touched.

Jan: Would you be happy accepting my CONFIG_{PV,HVM} patch in its silent
form for now?  If so, I will respin it.


I'd also like to us taking the effort to move as much as is suitable
into the pv/ and hvm/ subdirectories, to logically separate it from the
properly common code.

However, with the hypercall series, I was also taking the effort to
ensure that the code (particularly in pv/) was properly bool/const/mfn_t
and style correct, to try and break the cycle of uncertainty and poor
code quality.  For this, I was vaguely planning to clean the code up,
then move the file, rather than the other way around.

> This series is built on top of Roger's PVHv1 removal series.
>
> Wei Liu (5):
>   xen: move round_pg{up,down} to pfn.h
>   xen: include xen/types.h in domain.h
>   x86: split PV dom0 builder to domain_build_pv.c
>   x86: split PVH dom0 builder to domain_build_pv.c
>   x86: clean up header files in domain_build.c

I'd argue that these should be named differently.  They are specifically
hwdom_build.c, distinct from general domain building operations.

~Andrew

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

Re: [Xen-devel] [PATCH RFC 0/5] Refactor x86 domain builder

[Jan Beulich]

>>> On 03.03.17 at 11:56,  wrote:
> On 03/03/17 09:41, Wei Liu wrote:
> Jan: Would you be happy accepting my CONFIG_{PV,HVM} patch in its silent
> form for now?  If so, I will respin it.

I think so, yes.

>> This series is built on top of Roger's PVHv1 removal series.
>>
>> Wei Liu (5):
>>   xen: move round_pg{up,down} to pfn.h
>>   xen: include xen/types.h in domain.h
>>   x86: split PV dom0 builder to domain_build_pv.c
>>   x86: split PVH dom0 builder to domain_build_pv.c
>>   x86: clean up header files in domain_build.c
> 
> I'd argue that these should be named differently.  They are specifically
> hwdom_build.c, distinct from general domain building operations.

hwdom_build.c is wrong too - this is strictly Dom0, i.e. it ought
to be dom0_build.c.

Jan


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

Re: [Xen-devel] [PATCH RFC 3/5] x86: split PV dom0 builder to domain_build_pv.c

[Roger Pau Monné]

On Fri, Mar 03, 2017 at 09:41:09AM +, Wei Liu wrote:
> Long term we want to be able to disentangle PV and HVM code. Move the PV
> domain builder to a dedicated file.
> 
> This in turn requires exposing a few functions and variables via
> a new header domain_build.h.

I would add: "No functional change introduced", to make it clearer that it's
mostly just code movement.

> Signed-off-by: Wei Liu 
> ---
>  xen/arch/x86/Makefile  |   1 +
>  xen/arch/x86/domain_build.c| 894 +---
>  xen/arch/x86/domain_build.h|  32 ++

IMHO I would place this in include/asm-x86/, there are really no headers in
arch/x86/.

> diff --git a/xen/arch/x86/domain_build_pv.c b/xen/arch/x86/domain_build_pv.c

I would place this in arch/xen/x86/pv/domain_build.c

> new file mode 100644
> index 00..b4877c8dac
> --- /dev/null
> +++ b/xen/arch/x86/domain_build_pv.c
> @@ -0,0 +1,909 @@
> +/**
> + * domain_build_pv.c
> + */

You should probably keep Kier's copyright here, since this is all PV code.

Roger.

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

Re: [Xen-devel] [PATCH RFC 0/5] Refactor x86 domain builder

[Andrew Cooper]

On 03/03/17 10:56, Andrew Cooper wrote:
> On 03/03/17 09:41, Wei Liu wrote:
>> This series is built on top of Roger's PVHv1 removal series.
>>
>> Wei Liu (5):
>>   xen: move round_pg{up,down} to pfn.h
>>   xen: include xen/types.h in domain.h
>>   x86: split PV dom0 builder to domain_build_pv.c
>>   x86: split PVH dom0 builder to domain_build_pv.c
>>   x86: clean up header files in domain_build.c
> I'd argue that these should be named differently.  They are specifically
> hwdom_build.c, distinct from general domain building operations.

Actually on further thought, these are not even hwdom.  They are dom0
specific.

~Andrew

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

Re: [Xen-devel] [PATCH RFC 3/5] x86: split PV dom0 builder to domain_build_pv.c

[Jan Beulich]

>>> On 03.03.17 at 12:06,  wrote:
> On Fri, Mar 03, 2017 at 09:41:09AM +, Wei Liu wrote:
>> Long term we want to be able to disentangle PV and HVM code. Move the PV
>> domain builder to a dedicated file.
>> 
>> This in turn requires exposing a few functions and variables via
>> a new header domain_build.h.
> 
> I would add: "No functional change introduced", to make it clearer that it's
> mostly just code movement.
> 
>> Signed-off-by: Wei Liu 
>> ---
>>  xen/arch/x86/Makefile  |   1 +
>>  xen/arch/x86/domain_build.c| 894 
>> +---
>>  xen/arch/x86/domain_build.h|  32 ++
> 
> IMHO I would place this in include/asm-x86/, there are really no headers in
> arch/x86/.

That's the wrong criteria: If all consumers of a header are in the same
directory, putting it there is fine (even if it's the first one). In fact in
such a case I consider it worse to put it in asm-x86/, as that more
widely exposes such a supposedly limited scope header. And along
those lines I actually have plans to move some of the stuff currently
under include/asm-x86/ into (sub-trees of) arch/x86/.

However, with ...

>> diff --git a/xen/arch/x86/domain_build_pv.c b/xen/arch/x86/domain_build_pv.c
> 
> I would place this in arch/xen/x86/pv/domain_build.c

... this the case becomes less clear: Personally I'm not a fan of
#include "../file.h", so I wouldn't be certain which of the two is
the less undesirable place for the header.

Jan


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

Re: [Xen-devel] [PATCH RFC 4/5] x86: split PVH dom0 builder to domain_build_pv.c

[Roger Pau Monné]

On Fri, Mar 03, 2017 at 09:41:10AM +, Wei Liu wrote:
> Long term we want to be able to disentangle PV and HVM code. Move the
> PVH domain builder to a dedicated file. It will later depends on
> CONFIG_HVM or CONFIG_PVH.
> 
> This in turn requires exposing a few functions and variables via
> domain_build.h.

Again I would add something along the lines: "No functional changes expected".

> Signed-off-by: Wei Liu 
> ---
>  xen/arch/x86/Makefile   |1 +
>  xen/arch/x86/domain_build.c | 1068 +
>  xen/arch/x86/domain_build.h |8 +
>  xen/arch/x86/domain_build_pvh.c | 1097 
> +++

I would prefer that to be in arch/x86/hvm/.

> diff --git a/xen/arch/x86/domain_build_pvh.c b/xen/arch/x86/domain_build_pvh.c
> new file mode 100644
> index 00..6cd3ff2ef2
> --- /dev/null
> +++ b/xen/arch/x86/domain_build_pvh.c
> @@ -0,0 +1,1097 @@
> +/**
> + * domain_build_pvh.c
> + */

Since you are already touching this, and it's mostly my code, I would rather
prefer that you add the following header:

/**
 * arch/x86/hvm/domain_build.c
 *
 * PVH domain builder
 *
 * This program is free software; you can redistribute it and/or modify
 * it under the terms of the GNU General Public License as published by
 * the Free Software Foundation; either version 2 of the License, or
 * (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with this program; If not, see .
 *
 * Copyright (c) 2017 Citrix Systems Ltd.
 */

(I wouldn't mind licensing this under a dual GPL/BSD license, but I don't think
that's possible or useful inside of Xen).

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

[Xen-devel] [PATCH v2] x86/kconfig: Introduce CONFIG_PV and CONFIG_HVM

[Andrew Cooper]

Making PV and HVM guests individually compilable is useful as a reduction in
hypervisor size, and as an aid to enforcing clean API boundaries.

Introduce CONFIG_PV and CONFIG_HVM, although there is a lot of work to do
until either can actually be disabled.

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

v2:
 * Reduce to silent forms.  For build testing, manually invert the def_bool.
---
 xen/arch/x86/Kconfig  | 6 ++
 xen/arch/x86/Makefile | 4 ++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 96ca2bf..30c2769 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -32,6 +32,12 @@ menu "Architecture Features"
 
 source "arch/Kconfig"
 
+config PV
+   def_bool y
+
+config HVM
+   def_bool y
+
 config SHADOW_PAGING
 bool "Shadow Paging"
 default y
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index d6980b5..f75eca0 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -1,10 +1,10 @@
 subdir-y += acpi
 subdir-y += cpu
 subdir-y += genapic
-subdir-y += hvm
+subdir-$(CONFIG_HVM) += hvm
 subdir-y += mm
 subdir-$(CONFIG_XENOPROF) += oprofile
-subdir-y += pv
+subdir-$(CONFIG_PV) += pv
 subdir-y += x86_64
 
 alternative-y := alternative.init.o
-- 
2.1.4


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

[Xen-devel] Linking issue with latest git pull

[Praveen Kumar]

Hi,

Did a git pull and while building, I am getting below error :
Just FYI, i reconfigured and also did git clean -fd and rebuild
but the result is same.

Any pointer will be helpful, how to resolve this issue. Thanks in advance.

Error:

/usr/bin/ld: warning: libxendevicemodel.so.1, needed by
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so, not found (try using
-rpath or -rpath-link)
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference
to `xendevicemodel_map_pcidev_to_ioreq_server@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference
to `xendevicemodel_get_ioreq_server_info@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference
to `xendevicemodel_open@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference
to `xendevicemodel_unmap_io_range_from_ioreq_server@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference
to `xendevicemodel_modified_memory@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference
to `xendevicemodel_set_ioreq_server_state@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference
to `xendevicemodel_track_dirty_vram@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference
to `xendevicemodel_close@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference
to `xendevicemodel_set_mem_type@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference
to `xendevicemodel_destroy_ioreq_server@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference
to `xendevicemodel_set_pci_intx_level@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference
to `xendevicemodel_create_ioreq_server@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference
to `xendevicemodel_unmap_pcidev_from_ioreq_server@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference
to `xendevicemodel_inject_event@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference
to `xendevicemodel_set_pci_link_route@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference
to `xendevicemodel_inject_msi@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference
to `xendevicemodel_set_isa_irq_level@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference
to `xendevicemodel_map_io_range_to_ioreq_server@VERS_1.0'
collect2: error: ld returned 1 exit status
Makefile:740: recipe for target 'qemu-dm' failed
make[4]: *** [qemu-dm] Error 1
make[4]: Leaving directory '/home/praveen/xen/tools/qemu-
xen-traditional-dir-remote/i386-dm'
Makefile:42: recipe for target 'subdir-i386-dm' failed
make[3]: *** [subdir-i386-dm] Error 2
make[3]: Leaving directory '/home/praveen/xen/tools/qemu-
xen-traditional-dir-remote'
Makefile:201: recipe for target 'subdir-all-qemu-xen-traditional-dir' failed
make[2]: *** [subdir-all-qemu-xen-traditional-dir] Error 2
make[2]: Leaving directory '/home/praveen/xen/tools'
/home/praveen/xen/tools/../tools/Rules.mk:234: recipe for target
'subdirs-install' failed
make[1]: *** [subdirs-install] Error 2
make[1]: Leaving directory '/home/praveen/xen/tools'
Makefile:101: recipe for target 'install-tools' failed
make: *** [install-tools] Error 2

Regards,

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

Re: [Xen-devel] [PATCH 1/5] x86/vioapic: move domain out of hvm_vioapic struct

[Jan Beulich]

>>> On 23.02.17 at 12:52,  wrote:
> @@ -204,8 +205,7 @@ static void vioapic_write_indirect(
>  break;
>  }
>  
> -vioapic_write_redirent(
> -vioapic, redir_index, vioapic->ioregsel&1, val);
> +vioapic_write_redirent(d, redir_index, vioapic->ioregsel&1, val);

Please correct obvious coding style violations in cases like this. Of
course here it can be taken care of while committing.

> @@ -224,7 +224,7 @@ static int vioapic_write(
>  break;
>  
>  case VIOAPIC_REG_WINDOW:
> -vioapic_write_indirect(vioapic, val);
> +vioapic_write_indirect(v->domain, val);

I'll assume this will clarify itself with later patches: At this point it
feels wrong to lose the IO-APIC pointer in places like this, as from
domain and value alone you won't be able to reconstruct it. I.e.
I would have considered it more natural if you simply added
another function parameter.

I also wonder whether in at least some of the cases the new
struct domain * parameters of functions couldn't be const-
qualified.

In any event I'll want to have looked at more of this series to
understand whether the patch here is okay in its current form.

Jan


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

Re: [Xen-devel] [PATCH v2] x86/kconfig: Introduce CONFIG_PV and CONFIG_HVM

[Jan Beulich]

>>> On 03.03.17 at 12:22,  wrote:
> Making PV and HVM guests individually compilable is useful as a reduction in
> hypervisor size, and as an aid to enforcing clean API boundaries.
> 
> Introduce CONFIG_PV and CONFIG_HVM, although there is a lot of work to do
> until either can actually be disabled.
> 
> Signed-off-by: Andrew Cooper 

Acked-by: Jan Beulich 



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

[Xen-devel] [PATCH v3 4/5] configure: detect presence of libxendevicemodel

[Paul Durrant]

This patch adds code in configure to set CONFIG_XEN_CTRL_INTERFACE_VERSION
to a new value of 490 if libxendevicemodel is present in the build
environment.

Signed-off-by: Paul Durrant 
Reviewed-by: Anthony Perard 
---
Cc: Stefano Stabellini 
---
 configure | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/configure b/configure
index 8e8f18d..fc1e12b 100755
--- a/configure
+++ b/configure
@@ -1980,6 +1980,25 @@ EOF
   # Xen unstable
   elif
   cat > $TMPC <
+int main(void) {
+  xendevicemodel_handle *xd;
+
+  xd = xendevicemodel_open(0, 0);
+  xendevicemodel_close(xd);
+
+  return 0;
+}
+EOF
+  compile_prog "" "$xen_libs $xen_stable_libs -lxendevicemodel"
+then
+xen_stable_libs="$xen_stable_libs -lxendevicemodel"
+xen_ctrl_version=490
+xen=yes
+  elif
+  cat > $TMPC 

[Xen-devel] [PATCH v3 2/5] xen: rename xen_modified_memory() to xen_hvm_modified_memory()

[Paul Durrant]

This patch is a purely cosmetic change that avoids a name collision in
a subsequent patch.

Signed-off-by: Paul Durrant 
Reviewed-by: Anthony Perard 
---
Cc: Paolo Bonzini 
Cc: Stefano Stabellini 
---
 include/exec/ram_addr.h | 4 ++--
 include/hw/xen/xen.h| 2 +-
 xen-hvm-stub.c  | 2 +-
 xen-hvm.c   | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 3e79466..8715af6 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -259,7 +259,7 @@ static inline void 
cpu_physical_memory_set_dirty_range(ram_addr_t start,
 
 rcu_read_unlock();
 
-xen_modified_memory(start, length);
+xen_hvm_modified_memory(start, length);
 }
 
 #if !defined(_WIN32)
@@ -313,7 +313,7 @@ static inline void 
cpu_physical_memory_set_dirty_lebitmap(unsigned long *bitmap,
 
 rcu_read_unlock();
 
-xen_modified_memory(start, pages << TARGET_PAGE_BITS);
+xen_hvm_modified_memory(start, pages << TARGET_PAGE_BITS);
 } else {
 uint8_t clients = tcg_enabled() ? DIRTY_CLIENTS_ALL : 
DIRTY_CLIENTS_NOCODE;
 /*
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index 09c2ce5..2b1733b 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -43,7 +43,7 @@ void xen_hvm_init(PCMachineState *pcms, MemoryRegion 
**ram_memory);
 
 void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
struct MemoryRegion *mr, Error **errp);
-void xen_modified_memory(ram_addr_t start, ram_addr_t length);
+void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length);
 
 void xen_register_framebuffer(struct MemoryRegion *mr);
 
diff --git a/xen-hvm-stub.c b/xen-hvm-stub.c
index c500325..3ca6c51 100644
--- a/xen-hvm-stub.c
+++ b/xen-hvm-stub.c
@@ -50,7 +50,7 @@ void xen_register_framebuffer(MemoryRegion *mr)
 {
 }
 
-void xen_modified_memory(ram_addr_t start, ram_addr_t length)
+void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
 {
 }
 
diff --git a/xen-hvm.c b/xen-hvm.c
index dbb8c66..edf4983 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -1391,7 +1391,7 @@ void xen_shutdown_fatal_error(const char *fmt, ...)
 qemu_system_shutdown_request();
 }
 
-void xen_modified_memory(ram_addr_t start, ram_addr_t length)
+void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
 {
 if (unlikely(xen_in_migration)) {
 int rc;
-- 
2.1.4


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

[Xen-devel] [PATCH v3 5/5] xen: use libxendevicemodel when available

[Paul Durrant]

This patch modifies the wrapper functions in xen_common.h to use the
new xendevicemodel interface if it is available along with compatibility
code to use the old libxenctrl interface if it is not.

Signed-off-by: Paul Durrant 
Reviewed-by: Anthony Perard 
---
Cc: Stefano Stabellini 

NOTE: Xen patch 9970e98ace48 is needed to build

v3:
- Switch from macros to static inlines.

v2:
- Add a compat define for xenforeignmemory_close() since this is now
  used.
---
 include/hw/xen/xen_common.h | 199 
 xen-common.c|   8 ++
 2 files changed, 174 insertions(+), 33 deletions(-)

diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 31cf25f..740400a 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -9,6 +9,7 @@
 #undef XC_WANT_COMPAT_EVTCHN_API
 #undef XC_WANT_COMPAT_GNTTAB_API
 #undef XC_WANT_COMPAT_MAP_FOREIGN_API
+#undef XC_WANT_COMPAT_DEVICEMODEL_API
 
 #include 
 #include 
@@ -26,48 +27,179 @@ extern xc_interface *xen_xc;
  * We don't support Xen prior to 4.2.0.
  */
 
+#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 490
+
+typedef xc_interface xendevicemodel_handle;
+
+static inline xendevicemodel_handle *xendevicemodel_open(
+struct xentoollog_logger *logger, unsigned int open_flags)
+{
+return xen_xc;
+}
+
+static inline int xendevicemodel_create_ioreq_server(
+xendevicemodel_handle *dmod, domid_t domid, int handle_bufioreq,
+ioservid_t *id)
+{
+return xc_hvm_create_ioreq_server(dmod, domid, handle_bufioreq,
+  id);
+}
+
+static inline int xendevicemodel_get_ioreq_server_info(
+xendevicemodel_handle *dmod, domid_t domid, ioservid_t id,
+xen_pfn_t *ioreq_pfn, xen_pfn_t *bufioreq_pfn,
+evtchn_port_t *bufioreq_port)
+{
+return xc_hvm_get_ioreq_server_info(dmod, domid, id, ioreq_pfn,
+bufioreq_pfn, bufioreq_port);
+}
+
+static inline int xendevicemodel_map_io_range_to_ioreq_server(
+xendevicemodel_handle *dmod, domid_t domid, ioservid_t id, int is_mmio,
+uint64_t start, uint64_t end)
+{
+return xc_hvm_map_io_range_to_ioreq_server(dmod, domid, id, is_mmio,
+   start, end);
+}
+
+static inline int xendevicemodel_unmap_io_range_from_ioreq_server(
+xendevicemodel_handle *dmod, domid_t domid, ioservid_t id, int is_mmio,
+uint64_t start, uint64_t end)
+{
+return xc_hvm_unmap_io_range_from_ioreq_server(dmod, domid, id, is_mmio,
+   start, end);
+}
+
+static inline int xendevicemodel_map_pcidev_to_ioreq_server(
+xendevicemodel_handle *dmod, domid_t domid, ioservid_t id,
+uint16_t segment, uint8_t bus, uint8_t device, uint8_t function)
+{
+return xc_hvm_map_pcidev_to_ioreq_server(dmod, domid, id, segment,
+ bus, device, function);
+}
+
+static inline int xendevicemodel_unmap_pcidev_from_ioreq_server(
+xendevicemodel_handle *dmod, domid_t domid, ioservid_t id,
+uint16_t segment, uint8_t bus, uint8_t device, uint8_t function)
+{
+return xc_hvm_unmap_pcidev_from_ioreq_server(dmod, domid, id, segment,
+ bus, device, function);
+}
+
+static inline int xendevicemodel_destroy_ioreq_server(
+xendevicemodel_handle *dmod, domid_t domid, ioservid_t id)
+{
+return xc_hvm__destroy_ioreq_server(dmod, domid, id);
+}
+
+static inline int xendevicemodel_set_ioreq_server_state(
+xendevicemodel_handle *dmod, domid_t domid, ioservid_t id, int enabled)
+{
+return xc_hvm_set_ioreq_server_state(dmod, domid, id, enabled);
+}
+
+static inline int xendevicemodel_set_pci_intx_level(
+xendevicemodel_handle *dmod, domid_t domid, uint16_t segment,
+uint8_t bus, uint8_t device, uint8_t intx, unsigned int level)
+{
+return xc_hvm_set_pci_intx_level(dmod, domid, segment, bus, device,
+ intx, level);
+}
+
+static inline int xendevicemodel_set_isa_irq_level(
+xendevicemodel_handle *dmod, domid_t domid, uint8_t irq,
+unsigned int level)
+{
+return xc_hvm_set_isa_irq_level(dmod, domid, irq, level);
+}
+
+static inline int xendevicemodel_set_pci_link_route(
+xendevicemodel_handle *dmod, domid_t domid, uint8_t link, uint8_t irq)
+{
+return xc_hvm_set_pci_link_route(dmod, domid, link, irq);
+}
+
+static inline int xendevicemodel_inject_msi(
+xendevicemodel_handle *dmod, domid_t domid, uint64_t msi_addr,
+uint32_t msi_data)
+{
+return xc_hvm_inject_msi(dmod, domid, msi_addr, msi_data);
+}
+
+static inline int xendevicemodel_track_dirty_vram(
+xendevicemodel_handle *dmod, domid_t domid, uint64_t first_pfn,
+uint32_t nr, unsigned long *dirty_bitmap)
+{
+return xc_hvm_track_dirty_vram(dmod, domid, first_pfn, nr,
+   dirty_bitmap);
+}
+
+static inline int xendevicemodel_modified_memory(
+  

[Xen-devel] [PATCH v3 0/5] xen: use new xendevicemodel library

[Paul Durrant]

My recent patches to Xen [1] introduced a new library to support
running device models for HVM guests.
This series ports QEMU onto the new library if it is available in the
build environment.

[1] Patches starting with 
http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=b108240265deea37601f1a605910069a837da841

Paul Durrant (5):
  xen: make use of xen_xc implicit in xen_common.h inlines
  xen: rename xen_modified_memory() to xen_hvm_modified_memory()
  xen: create wrappers for all other uses of xc_hvm_XXX() functions
  configure: detect presence of libxendevicemodel
  xen: use libxendevicemodel when available

 configure|  19 +++
 hw/i386/xen/xen_platform.c   |   2 +-
 hw/xen/xen_backend.c |   2 -
 include/exec/ram_addr.h  |   4 +-
 include/hw/xen/xen.h |   2 +-
 include/hw/xen/xen_backend.h |   2 -
 include/hw/xen/xen_common.h  | 285 +++
 xen-common.c |  11 ++
 xen-hvm-stub.c   |   2 +-
 xen-hvm.c|  49 
 10 files changed, 292 insertions(+), 86 deletions(-)

-- 
2.1.4


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

[Xen-devel] [PATCH v3 3/5] xen: create wrappers for all other uses of xc_hvm_XXX() functions

[Paul Durrant]

This patch creates inline wrapper functions in xen_common.h for all open
coded calls to xc_hvm_XXX() functions outside of xen_common.h so that use
of xen_xc can be made implicit. This again is in preparation for the move
to using libxendevicemodel.

Signed-off-by: Paul Durrant 
Reviewed-by: Anthony Perard 
---
Cc: Stefano Stabellini 
Cc: Paolo Bonzini 
Cc: Richard Henderson 
Cc: Eduardo Habkost 
Cc: "Michael S. Tsirkin" 
---
 hw/i386/xen/xen_platform.c  |  2 +-
 include/hw/xen/xen_common.h | 44 
 xen-hvm.c   | 27 +--
 3 files changed, 58 insertions(+), 15 deletions(-)

diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
index 6010f35..1419fc9 100644
--- a/hw/i386/xen/xen_platform.c
+++ b/hw/i386/xen/xen_platform.c
@@ -195,7 +195,7 @@ static void platform_fixed_ioport_writeb(void *opaque, 
uint32_t addr, uint32_t v
 case 0: /* Platform flags */ {
 hvmmem_type_t mem_type = (val & PFFLAG_ROM_LOCK) ?
 HVMMEM_ram_ro : HVMMEM_ram_rw;
-if (xc_hvm_set_mem_type(xen_xc, xen_domid, mem_type, 0xc0, 0x40)) {
+if (xen_set_mem_type(xen_domid, mem_type, 0xc0, 0x40)) {
 DPRINTF("unable to change ro/rw state of ROM memory area!\n");
 } else {
 s->flags = val & PFFLAG_ROM_LOCK;
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 1e08b98..31cf25f 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -26,6 +26,50 @@ extern xc_interface *xen_xc;
  * We don't support Xen prior to 4.2.0.
  */
 
+static inline int xen_set_mem_type(domid_t domid, hvmmem_type_t type,
+   uint64_t first_pfn, uint32_t nr)
+{
+return xc_hvm_set_mem_type(xen_xc, domid, type, first_pfn, nr);
+}
+
+static inline int xen_set_pci_intx_level(domid_t domid, uint16_t segment,
+ uint8_t bus, uint8_t device,
+ uint8_t intx, unsigned int level)
+{
+return xc_hvm_set_pci_intx_level(xen_xc, domid, segment, bus, device,
+ intx, level);
+}
+
+static inline int xen_set_pci_link_route(domid_t domid, uint8_t link,
+ uint8_t irq)
+{
+return xc_hvm_set_pci_link_route(xen_xc, domid, link, irq);
+}
+
+static inline int xen_inject_msi(domid_t domid, uint64_t msi_addr,
+ uint32_t msi_data)
+{
+return xc_hvm_inject_msi(xen_xc, domid, msi_addr, msi_data);
+}
+
+static inline int xen_set_isa_irq_level(domid_t domid, uint8_t irq,
+unsigned int level)
+{
+return xc_hvm_set_isa_irq_level(xen_xc, domid, irq, level);
+}
+
+static inline int xen_track_dirty_vram(domid_t domid, uint64_t first_pfn,
+   uint32_t nr, unsigned long *bitmap)
+{
+return xc_hvm_track_dirty_vram(xen_xc, domid, first_pfn, nr, bitmap);
+}
+
+static inline int xen_modified_memory(domid_t domid, uint64_t first_pfn,
+  uint32_t nr)
+{
+return xc_hvm_modified_memory(xen_xc, domid, first_pfn, nr);
+}
+
 /* Xen 4.2 through 4.6 */
 #if CONFIG_XEN_CTRL_INTERFACE_VERSION < 471
 
diff --git a/xen-hvm.c b/xen-hvm.c
index edf4983..4b928cf 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -125,8 +125,8 @@ int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
 
 void xen_piix3_set_irq(void *opaque, int irq_num, int level)
 {
-xc_hvm_set_pci_intx_level(xen_xc, xen_domid, 0, 0, irq_num >> 2,
-  irq_num & 3, level);
+xen_set_pci_intx_level(xen_domid, 0, 0, irq_num >> 2,
+   irq_num & 3, level);
 }
 
 void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len)
@@ -141,7 +141,7 @@ void xen_piix_pci_write_config_client(uint32_t address, 
uint32_t val, int len)
 }
 v &= 0xf;
 if (((address + i) >= 0x60) && ((address + i) <= 0x63)) {
-xc_hvm_set_pci_link_route(xen_xc, xen_domid, address + i - 0x60, 
v);
+xen_set_pci_link_route(xen_domid, address + i - 0x60, v);
 }
 }
 }
@@ -156,7 +156,7 @@ int xen_is_pirq_msi(uint32_t msi_data)
 
 void xen_hvm_inject_msi(uint64_t addr, uint32_t data)
 {
-xc_hvm_inject_msi(xen_xc, xen_domid, addr, data);
+xen_inject_msi(xen_domid, addr, data);
 }
 
 static void xen_suspend_notifier(Notifier *notifier, void *data)
@@ -168,7 +168,7 @@ static void xen_suspend_notifier(Notifier *notifier, void 
*data)
 
 static void xen_set_irq(void *opaque, int irq, int level)
 {
-xc_hvm_set_isa_irq_level(xen_xc, xen_domid, irq, level);
+xen_set_isa_irq_level(xen_domid, irq, level);
 }
 
 qemu_irq *xen_interrupt_controller_init(void)
@@ -481,10 +481,10 @@ static void xen_set_memory(struct MemoryListener 
*listener,
section->mr, section->offset_within_region);
 }

[Xen-devel] [PATCH v3 1/5] xen: make use of xen_xc implicit in xen_common.h inlines

[Paul Durrant]

Doing this will make the transition to using the new libxendevicemodel
interface less intrusive on the callers of these functions, since using
the new library will require a change of handle.

NOTE: The patch also moves the 'externs' for xen_xc and xen_fmem from
  xen_backend.h to xen_common.h, and the declarations from
  xen_backend.c to xen-common.c, which is where they belong.

Signed-off-by: Paul Durrant 
Reviewed-by: Anthony Perard 
---
Cc: Stefano Stabellini 
---
 hw/xen/xen_backend.c |  2 -
 include/hw/xen/xen_backend.h |  2 -
 include/hw/xen/xen_common.h  | 90 +++-
 xen-common.c |  3 ++
 xen-hvm.c| 20 +-
 5 files changed, 60 insertions(+), 57 deletions(-)

diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
index 6c21c37..d34c49e 100644
--- a/hw/xen/xen_backend.c
+++ b/hw/xen/xen_backend.c
@@ -43,8 +43,6 @@ BusState *xen_sysbus;
 /* - */
 
 /* public */
-xc_interface *xen_xc = NULL;
-xenforeignmemory_handle *xen_fmem = NULL;
 struct xs_handle *xenstore = NULL;
 const char *xen_protocol;
 
diff --git a/include/hw/xen/xen_backend.h b/include/hw/xen/xen_backend.h
index 4f4799a..30811a1 100644
--- a/include/hw/xen/xen_backend.h
+++ b/include/hw/xen/xen_backend.h
@@ -14,8 +14,6 @@
 OBJECT_CHECK(XenDevice, (obj), TYPE_XENBACKEND)
 
 /* variables */
-extern xc_interface *xen_xc;
-extern xenforeignmemory_handle *xen_fmem;
 extern struct xs_handle *xenstore;
 extern const char *xen_protocol;
 extern DeviceState *xen_sysdev;
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index dce76ee..1e08b98 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -20,6 +20,8 @@
 #include "qemu/queue.h"
 #include "hw/xen/trace.h"
 
+extern xc_interface *xen_xc;
+
 /*
  * We don't support Xen prior to 4.2.0.
  */
@@ -73,6 +75,8 @@ static inline void *xenforeignmemory_map(xc_interface *h, 
uint32_t dom,
 
 #endif
 
+extern xenforeignmemory_handle *xen_fmem;
+
 void destroy_hvm_domain(bool reboot);
 
 /* shutdown/destroy current domain because of an error */
@@ -107,8 +111,7 @@ static inline int xen_get_vmport_regs_pfn(xc_interface *xc, 
domid_t dom,
 
 #endif
 
-static inline int xen_get_default_ioreq_server_info(xc_interface *xc,
-domid_t dom,
+static inline int xen_get_default_ioreq_server_info(domid_t dom,
 xen_pfn_t *ioreq_pfn,
 xen_pfn_t *bufioreq_pfn,
 evtchn_port_t
@@ -117,7 +120,7 @@ static inline int 
xen_get_default_ioreq_server_info(xc_interface *xc,
 unsigned long param;
 int rc;
 
-rc = xc_get_hvm_param(xc, dom, HVM_PARAM_IOREQ_PFN, ¶m);
+rc = xc_get_hvm_param(xen_xc, dom, HVM_PARAM_IOREQ_PFN, ¶m);
 if (rc < 0) {
 fprintf(stderr, "failed to get HVM_PARAM_IOREQ_PFN\n");
 return -1;
@@ -125,7 +128,7 @@ static inline int 
xen_get_default_ioreq_server_info(xc_interface *xc,
 
 *ioreq_pfn = param;
 
-rc = xc_get_hvm_param(xc, dom, HVM_PARAM_BUFIOREQ_PFN, ¶m);
+rc = xc_get_hvm_param(xen_xc, dom, HVM_PARAM_BUFIOREQ_PFN, ¶m);
 if (rc < 0) {
 fprintf(stderr, "failed to get HVM_PARAM_BUFIOREQ_PFN\n");
 return -1;
@@ -133,7 +136,7 @@ static inline int 
xen_get_default_ioreq_server_info(xc_interface *xc,
 
 *bufioreq_pfn = param;
 
-rc = xc_get_hvm_param(xc, dom, HVM_PARAM_BUFIOREQ_EVTCHN,
+rc = xc_get_hvm_param(xen_xc, dom, HVM_PARAM_BUFIOREQ_EVTCHN,
   ¶m);
 if (rc < 0) {
 fprintf(stderr, "failed to get HVM_PARAM_BUFIOREQ_EVTCHN\n");
@@ -156,63 +159,64 @@ static inline int 
xen_get_default_ioreq_server_info(xc_interface *xc,
 
 typedef uint16_t ioservid_t;
 
-static inline void xen_map_memory_section(xc_interface *xc, domid_t dom,
+static inline void xen_map_memory_section(domid_t dom,
   ioservid_t ioservid,
   MemoryRegionSection *section)
 {
 }
 
-static inline void xen_unmap_memory_section(xc_interface *xc, domid_t dom,
+static inline void xen_unmap_memory_section(domid_t dom,
 ioservid_t ioservid,
 MemoryRegionSection *section)
 {
 }
 
-static inline void xen_map_io_section(xc_interface *xc, domid_t dom,
+static inline void xen_map_io_section(domid_t dom,
   ioservid_t ioservid,
   MemoryRegionSection *section)
 {
 }
 
-static inline void xen_unmap_io_section(xc_interface *xc, domid_t dom,
+static inline void xen_unmap_io_section(domid_t dom,
 ioservid_t ioservid,
 MemoryRegi

Re: [Xen-devel] [PATCH v9 3/8] VMX: Properly handle pi when all the assigned devices are removed

[Tian, Kevin]

> From: Gao, Chao
> Sent: Monday, February 27, 2017 9:46 AM
> 
> From: Feng Wu 
> 
> This patch handles some corner cases when the last assigned device
> is removed from the domain. In this case we should carefully handle
> pi descriptor and the per-cpu blocking list, to make sure:
> - all the PI descriptor are in the right state when next time a
> devices is assigned to the domain again.
> - No remaining vcpus of the domain in the per-cpu blocking list.
> 
> Here we call vmx_pi_unblock_vcpu() to remove the vCPU from the blocking list
> if it is on the list. However, this could happen when vmx_vcpu_block() is
> being called, hence we might incorrectly add the vCPU to the blocking list
> while the last devcie is detached from the domain. Consider that the situation
> can only occur when detaching the last device from the domain and it is not
> a frequent operation, so we use domain_pause before that, which is considered
> as an clean and maintainable solution for the situation.
> 
> Signed-off-by: Feng Wu 
> Signed-off-by: Chao Gao 
> Reviewed-by: Jan Beulich 

Acked-by: Kevin Tian 

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

Re: [Xen-devel] [PATCH v9 1/8] VMX: Permanently assign PI hook vmx_pi_switch_to()

[Tian, Kevin]

> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: Friday, March 03, 2017 6:49 PM
> 
> >>> On 03.03.17 at 09:29,  wrote:
> >>  From: Jan Beulich [mailto:jbeul...@suse.com]
> >> Sent: Wednesday, March 01, 2017 3:42 PM
> >> Sounds good to me (read: Reviewed-by: Jan Beulich ).
> >> If Kevin would give his ack, I could replace the comment while committing,
> >> so you wouldn't need to re-send.
> >
> > Good to me too.
> >
> > Acked-by: Kevin Tian 
> 
> How about patch 3 of this series then? Patch 4 already has your ack,
> and patches 5 and onwards need another iteration afaict.
> 

I thought I did but looks not. Just gave my ack. For patch 5 onwards
I'll look the new iteration closely after my vacation.

Thanks
Kevin

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

[Xen-devel] [ovmf test] 106390: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemuu-ovmf-amd64 9 debian-hvm-install fail REGR. vs. 105963
 test-amd64-amd64-xl-qemuu-ovmf-amd64 9 debian-hvm-install fail REGR. vs. 105963

version targeted for testing:
 ovmf a11928f3310518ab1c6fd34e8d0fdbb72de9602c
baseline version:
 ovmf e0307a7dad02aa8c0cd8b3b0b9edce8ddb3fef2e

Last test of basis   105963  2017-02-21 21:43:31 Z9 days
Failing since105980  2017-02-22 10:03:53 Z9 days   24 attempts
Testing same since   106378  2017-03-02 18:45:27 Z0 days2 attempts


People who touched revisions under test:
  Anthony PERARD 
  Ard Biesheuvel 
  Brijesh Singh 
  Chen A Chen 
  Dandan Bi 
  edk2-devel On Behalf Of rthomaiy <[mailto:edk2-devel-boun...@lists.01.org]>
  Fu Siyuan 
  Hao Wu 
  Hegde Nagaraj P 
  Jeff Fan 
  Jiaxin Wu 
  Jiewen Yao 
  Laszlo Ersek 
  Leo Duran 
  Paolo Bonzini 
  Qin Long 
  Richard Thomaiyar 
  Ruiyu Ni 
  Star Zeng 
  Wu Jiaxin 
  Yonghong Zhu 
  Zhang Lubo 

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 fail
 test-amd64-i386-xl-qemuu-ovmf-amd64  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.

(No revision log; it would be 1964 lines long.)

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

Re: [Xen-devel] Linking issue with latest git pull

[Paul Durrant]

Please don't post HTML...

---
From: Xen-devel [mailto:xen-devel-boun...@lists.xen.org] On Behalf Of Praveen 
Kumar
Sent: 03 March 2017 11:23
To: xen-de...@lists.xenproject.org
Subject: [Xen-devel] Linking issue with latest git pull

Hi,

Did a git pull and while building, I am getting below error :
Just FYI, i reconfigured and also did git clean -fd and rebuild but 
the result is same.

Any pointer will be helpful, how to resolve this issue. Thanks in advance.

Error:

/usr/bin/ld: warning: libxendevicemodel.so.1, needed by 
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so, not found (try using 
-rpath or -rpath-link)
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference to 
`xendevicemodel_map_pcidev_to_ioreq_server@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference to 
`xendevicemodel_get_ioreq_server_info@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference to 
`xendevicemodel_open@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference to 
`xendevicemodel_unmap_io_range_from_ioreq_server@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference to 
`xendevicemodel_modified_memory@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference to 
`xendevicemodel_set_ioreq_server_state@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference to 
`xendevicemodel_track_dirty_vram@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference to 
`xendevicemodel_close@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference to 
`xendevicemodel_set_mem_type@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference to 
`xendevicemodel_destroy_ioreq_server@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference to 
`xendevicemodel_set_pci_intx_level@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference to 
`xendevicemodel_create_ioreq_server@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference to 
`xendevicemodel_unmap_pcidev_from_ioreq_server@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference to 
`xendevicemodel_inject_event@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference to 
`xendevicemodel_set_pci_link_route@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference to 
`xendevicemodel_inject_msi@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference to 
`xendevicemodel_set_isa_irq_level@VERS_1.0'
/home/praveen/xen/tools/../tools/libxc/libxenctrl.so: undefined reference to 
`xendevicemodel_map_io_range_to_ioreq_server@VERS_1.0'
collect2: error: ld returned 1 exit status
Makefile:740: recipe for target 'qemu-dm' failed
make[4]: *** [qemu-dm] Error 1
make[4]: Leaving directory 
'/home/praveen/xen/tools/qemu-xen-traditional-dir-remote/i386-dm'
Makefile:42: recipe for target 'subdir-i386-dm' failed
make[3]: *** [subdir-i386-dm] Error 2
make[3]: Leaving directory 
'/home/praveen/xen/tools/qemu-xen-traditional-dir-remote'
Makefile:201: recipe for target 'subdir-all-qemu-xen-traditional-dir' failed
make[2]: *** [subdir-all-qemu-xen-traditional-dir] Error 2
make[2]: Leaving directory '/home/praveen/xen/tools'
/home/praveen/xen/tools/../tools/Rules.mk:234: recipe for target 
'subdirs-install' failed
make[1]: *** [subdirs-install] Error 2
make[1]: Leaving directory '/home/praveen/xen/tools'
Makefile:101: recipe for target 'install-tools' failed
make: *** [install-tools] Error 2 
---

As I said on the thread starting at 
https://lists.xen.org/archives/html/xen-devel/2017-03/msg00256.html the problem 
is that you need patches:


http://xenbits.xen.org/gitweb/?p=qemu-xen-traditional.git;a=commit;h=58b9047bf2da88d2976bd1b7ba50dfdcc68b503d
http://xenbits.xen.org/gitweb/?p=qemu-xen-traditional.git;a=commit;h=8b4834ee1202852ed83a9fc61268c65fb6961ea7

So, however you achieve an update to you qemu-xen-traditional repo, you need to 
make sure those patches are present.

  Paul

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

Re: [Xen-devel] [PATCH 2/5] x86/vioapic: allow the vIO APIC to have a variable number of pins

[Jan Beulich]

>>> On 23.02.17 at 12:52,  wrote:
> @@ -424,40 +424,141 @@ void vioapic_update_EOI(struct domain *d, u8 vector)
>  spin_unlock(&d->arch.hvm_domain.irq_lock);
>  }
>  
> -static int ioapic_save(struct domain *d, hvm_domain_context_t *h)
> +#define VIOAPIC_SAVE_CONST offsetof(struct hvm_hw_vioapic, redirtbl)

What is this redirtbl field, which oddly enough is a pointer (not allowed
in the public interface) good for? I can't seem to find any use of it
other than as marker (for use with offsetof()). With all the
complications here and below plus the fixed number of entries
remaining to be fixed for actual migration purposes I wonder if you
wouldn't be better off by keeping the structure mostly as is, simply
converting the redirtbl[] field to a union (guarded by a __XEN__
conditional) containing both a fixed size array and a variable size
one (or to be precise, a zero size one, as a variable size one is not
allowed there).

Another alternative would be to introduce a Xen internal sibling
struct with a variable size array, and with all fields properly build-
time-asserted to be identical between both.

I'll skip the rest of the patch assuming much of it could be dropped
this way.

Jan


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

Re: [Xen-devel] [PATCH 01/10] x86: assembly, ENTRY for fn, GLOBAL for data

[Jiri Slaby]

On 03/01/2017, 11:27 AM, Ingo Molnar wrote:
> But no strong feelings either way, I just try to sneak in these small 
> namespace 
> structure tricks when nobody's looking! ;-)

So I assume to introduce two underscores.

thanks,
-- 
js
suse labs

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

[Xen-devel] [linux-linus test] 106388: regressions - FAIL

[osstest service owner]

flight 106388 linux-linus real [real]
http://logs.test-lab.xenproject.org/osstest/logs/106388/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail REGR. vs. 59254
 test-armhf-armhf-xl  11 guest-start   fail REGR. vs. 59254
 test-armhf-armhf-xl-multivcpu 11 guest-start  fail REGR. vs. 59254
 test-armhf-armhf-xl-credit2  11 guest-start   fail REGR. vs. 59254
 test-armhf-armhf-xl-xsm  11 guest-start   fail REGR. vs. 59254
 test-armhf-armhf-xl-arndale  11 guest-start   fail REGR. vs. 59254
 test-armhf-armhf-libvirt-xsm 11 guest-start   fail REGR. vs. 59254
 test-armhf-armhf-xl-cubietruck 11 guest-start fail REGR. vs. 59254
 test-armhf-armhf-libvirt 11 guest-start   fail REGR. vs. 59254

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds 11 guest-start   fail REGR. vs. 59254
 test-amd64-amd64-xl-rtds  9 debian-installfail REGR. vs. 59254
 test-armhf-armhf-xl-vhd   9 debian-di-install   fail baseline untested
 test-armhf-armhf-libvirt-raw  9 debian-di-install   fail baseline untested
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 59254
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 59254
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 59254
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 59254

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-rtds  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 build-arm64-xsm   5 xen-buildfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 build-arm64   5 xen-buildfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass

version targeted for testing:
 linuxbbe08c0a43e2c5ee3a00de68c0e867a08a9aa990
baseline version:
 linux45820c294fe1b1a9df495d57f40585ef2d069a39

Last test of basis59254  2015-07-09 04:20:48 Z  603 days
Failing since 59348  2015-07-10 04:24:05 Z  602 days  308 attempts
Testing same since   106388  2017-03-03 02:17:37 Z0 days1 attempts


8030 people touched revisions under test,
not listing them all

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  fail
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  fail
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-arm64-libvirt  blocked 
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-arm64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 build-amd64-rumprun  pass
 build-i386-rump

[Xen-devel] [PATCH v4 0/4] x86: remove PVHv1

[Roger Pau Monne]

Hello,

This patch series removes the PVHv1 code, both from the hypervisor and the
tools, and also gets rid of the has_hvm_container_{domain/vcpu} macro, since
from Xen's point of view there are only two types of guests: PV or HVM.

Last patch is a minor code movement to have all the domain build PVH related
functions together.

This revision introduces the PVH domain type to libxl, and gets rid of the
"none" device_model_version and the "pvh" field in the libxl create info
struct. There's also a pre-requisite patch (#1) for the ocaml IDL generator.

As usual, the whole series can be found on my git tree at:

git://xenbits.xen.org/people/royger/xen.git rm_pvh1_v3.1

Thanks, Roger.


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

[Xen-devel] [PATCH v3 1/4] ocaml/gen: fix ocaml type/code generator from IDL

[Roger Pau Monne]

From: Ian Jackson 

This patch adds support for union members which have their own type name.

Signed-off-by: Ian Jackson 
---
This is a pre-requisite for the PVHv1 removal patch.
---
Cc: David Scott 
Cc: Ian Jackson 
Cc: Wei Liu 
---
 tools/ocaml/libs/xl/genwrap.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/ocaml/libs/xl/genwrap.py b/tools/ocaml/libs/xl/genwrap.py
index 1c8ad81..9a65d73 100644
--- a/tools/ocaml/libs/xl/genwrap.py
+++ b/tools/ocaml/libs/xl/genwrap.py
@@ -155,7 +155,7 @@ def gen_ocaml_keyedunions(ty, interface, indent, parent = 
None):
 u.append("%s" % (f.name.capitalize()))
 elif isinstance(f.type, idl.Struct):
 if f.type.rawname is not None:
-u.append("%s of %s" % (f.name.capitalize(), 
f.type.rawname.capitalize()))
+u.append("%s of %s.t" % (f.name.capitalize(), 
f.type.rawname.capitalize()))
 elif f.type.has_fields():
 u.append("%s of %s_%s" % (f.name.capitalize(), nparent, 
f.name))
 else:
@@ -325,7 +325,7 @@ def c_val(ty, c, o, indent="", parent = None):
 s += "\t\tcase %d:\n" % (n)
 s += "\t\t%s = %s;\n" % (parent + ty.keyvar.name, 
f.enumname)
 (nparent,fexpr) = ty.member(c, f, False)
-s += "%s" % c_val(f.type, fexpr, "Field(%s, 0)" % o, 
indent=indent+"\t\t")
+s += "%s" % c_val(f.type, fexpr, "Field(%s, 0)" % o, 
parent=nparent, indent=indent+"\t\t")
 s += "break;\n"
 n += 1
 s += "\t\tdefault: failwith_xl(ERROR_FAIL, \"variant handling bug 
%s%s (block)\"); break;\n" % (parent, ty.keyvar.name)
-- 
2.10.1 (Apple Git-78)


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

[Xen-devel] [PATCH v3 4/4] x86/PVHv2: move pvh_setup_e820 together with the other pvh functions

[Roger Pau Monne]

This function is only used by PVHv2 domain build, so move it together with the
other PVH domain build functions.

Just code motion, no functional change.

Signed-off-by: Roger Pau Monné 
Acked-by: Jan Beulich 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
---
 xen/arch/x86/domain_build.c | 134 ++--
 1 file changed, 67 insertions(+), 67 deletions(-)

diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
index a0d9ee0..e4b60d4 100644
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -470,73 +470,6 @@ static void __init process_dom0_ioports_disable(struct 
domain *dom0)
 }
 }
 
-static __init void pvh_setup_e820(struct domain *d, unsigned long nr_pages)
-{
-struct e820entry *entry, *entry_guest;
-unsigned int i;
-unsigned long pages, cur_pages = 0;
-uint64_t start, end;
-
-/*
- * Craft the e820 memory map for Dom0 based on the hardware e820 map.
- */
-d->arch.e820 = xzalloc_array(struct e820entry, e820.nr_map);
-if ( !d->arch.e820 )
-panic("Unable to allocate memory for Dom0 e820 map");
-entry_guest = d->arch.e820;
-
-/* Clamp e820 memory map to match the memory assigned to Dom0 */
-for ( i = 0, entry = e820.map; i < e820.nr_map; i++, entry++ )
-{
-if ( entry->type != E820_RAM )
-{
-*entry_guest = *entry;
-goto next;
-}
-
-if ( nr_pages == cur_pages )
-{
-/*
- * We already have all the assigned memory,
- * skip this entry
- */
-continue;
-}
-
-/*
- * Make sure the start and length are aligned to PAGE_SIZE, because
- * that's the minimum granularity of the 2nd stage translation. Since
- * the p2m code uses PAGE_ORDER_4K internally, also use it here in
- * order to prevent this code from getting out of sync.
- */
-start = ROUNDUP(entry->addr, PAGE_SIZE << PAGE_ORDER_4K);
-end = (entry->addr + entry->size) &
-  ~((PAGE_SIZE << PAGE_ORDER_4K) - 1);
-if ( start >= end )
-continue;
-
-entry_guest->type = E820_RAM;
-entry_guest->addr = start;
-entry_guest->size = end - start;
-pages = PFN_DOWN(entry_guest->size);
-if ( (cur_pages + pages) > nr_pages )
-{
-/* Truncate region */
-entry_guest->size = (nr_pages - cur_pages) << PAGE_SHIFT;
-cur_pages = nr_pages;
-}
-else
-{
-cur_pages += pages;
-}
- next:
-d->arch.nr_e820++;
-entry_guest++;
-}
-ASSERT(cur_pages == nr_pages);
-ASSERT(d->arch.nr_e820 <= e820.nr_map);
-}
-
 static __init void dom0_update_physmap(struct domain *d, unsigned long pfn,
unsigned long mfn, unsigned long vphysmap_s)
 {
@@ -1685,6 +1618,73 @@ static void __init pvh_steal_low_ram(struct domain *d, 
unsigned long start,
 }
 }
 
+static __init void pvh_setup_e820(struct domain *d, unsigned long nr_pages)
+{
+struct e820entry *entry, *entry_guest;
+unsigned int i;
+unsigned long pages, cur_pages = 0;
+uint64_t start, end;
+
+/*
+ * Craft the e820 memory map for Dom0 based on the hardware e820 map.
+ */
+d->arch.e820 = xzalloc_array(struct e820entry, e820.nr_map);
+if ( !d->arch.e820 )
+panic("Unable to allocate memory for Dom0 e820 map");
+entry_guest = d->arch.e820;
+
+/* Clamp e820 memory map to match the memory assigned to Dom0 */
+for ( i = 0, entry = e820.map; i < e820.nr_map; i++, entry++ )
+{
+if ( entry->type != E820_RAM )
+{
+*entry_guest = *entry;
+goto next;
+}
+
+if ( nr_pages == cur_pages )
+{
+/*
+ * We already have all the assigned memory,
+ * skip this entry
+ */
+continue;
+}
+
+/*
+ * Make sure the start and length are aligned to PAGE_SIZE, because
+ * that's the minimum granularity of the 2nd stage translation. Since
+ * the p2m code uses PAGE_ORDER_4K internally, also use it here in
+ * order to prevent this code from getting out of sync.
+ */
+start = ROUNDUP(entry->addr, PAGE_SIZE << PAGE_ORDER_4K);
+end = (entry->addr + entry->size) &
+  ~((PAGE_SIZE << PAGE_ORDER_4K) - 1);
+if ( start >= end )
+continue;
+
+entry_guest->type = E820_RAM;
+entry_guest->addr = start;
+entry_guest->size = end - start;
+pages = PFN_DOWN(entry_guest->size);
+if ( (cur_pages + pages) > nr_pages )
+{
+/* Truncate region */
+entry_guest->size = (nr_pages - cur_pages) << PAGE_SHIFT;
+cur_pages = nr_pages;
+}
+else
+{
+cur_pages += pages;
+}
+ next:

[Xen-devel] [PATCH v3 2/4] x86: remove PVHv1 code

[Roger Pau Monne]

This removal applies to both the hypervisor and the toolstack side of PVHv1.

Note that on the toolstack side a new PVH domain type is introduced to libxl.
The "none" device model version is removed, together with the "pvh" field in
the create info struct (the defines announcing those features are also removed
from libxl.h). The canonical way to create a PVH guest in libxl is to add
"pvh=1" to the guest config file.

Signed-off-by: Roger Pau Monné 
Reviewed-by: Andrew Cooper 
Acked-by: George Dunlap 
Reviewed-by: Paul Durrant 
Acked-by: Elena Ufimtseva 
Reviewed-by: Kevin Tian 
---
Changes since v1:
 - Remove dom0pvh option from the command line docs.
 - Bump domctl interface version due to the removed CDF flag.
 - Introduce LIBXL_DOMAIN_TYPE_PVH.
 - Remove "none" from the valid device model version options.
 - Update the xl.cfg(5) man page to reflect the changes.

---
Cc: Ian Jackson 
Cc: Wei Liu 
Cc: Elena Ufimtseva 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Paul Durrant 
Cc: Jun Nakajima 
Cc: Kevin Tian 
Cc: George Dunlap 
Cc: Razvan Cojocaru 
Cc: Tamas K Lengyel 
---
 docs/man/xl.cfg.pod.5.in|  16 +-
 docs/misc/pvh-readme.txt|  63 
 docs/misc/xen-command-line.markdown |   7 -
 tools/debugger/gdbsx/xg/xg_main.c   |   4 +-
 tools/libxc/include/xc_dom.h|   1 -
 tools/libxc/include/xenctrl.h   |   2 +-
 tools/libxc/xc_cpuid_x86.c  |  13 +-
 tools/libxc/xc_dom_core.c   |   9 --
 tools/libxc/xc_dom_x86.c|  49 +++---
 tools/libxc/xc_domain.c |   1 -
 tools/libxl/libxl.h |  22 +--
 tools/libxl/libxl_console.c |   1 +
 tools/libxl/libxl_create.c  |  64 +++-
 tools/libxl/libxl_disk.c|  10 +-
 tools/libxl/libxl_dm.c  |   2 +
 tools/libxl/libxl_dom.c |  86 ++-
 tools/libxl/libxl_dom_save.c|   7 +-
 tools/libxl/libxl_dom_suspend.c |   4 +-
 tools/libxl/libxl_domain.c  |  18 +--
 tools/libxl/libxl_internal.h|   1 -
 tools/libxl/libxl_mem.c |   1 +
 tools/libxl/libxl_nic.c |   7 +-
 tools/libxl/libxl_pci.c |   9 +-
 tools/libxl/libxl_stream_read.c |   8 +-
 tools/libxl/libxl_stream_write.c|  14 +-
 tools/libxl/libxl_types.idl | 115 ---
 tools/libxl/libxl_usb.c |   4 +-
 tools/libxl/libxl_x86.c |  31 ++--
 tools/libxl/libxl_x86_acpi.c|   3 +-
 tools/xl/xl_parse.c |   8 +-
 xen/arch/x86/cpu/vpmu.c |   3 +-
 xen/arch/x86/domain.c   |  42 +-
 xen/arch/x86/domain_build.c | 287 +---
 xen/arch/x86/domctl.c   |   7 +-
 xen/arch/x86/hvm/hvm.c  |  81 +-
 xen/arch/x86/hvm/hypercall.c|   4 +-
 xen/arch/x86/hvm/io.c   |   2 -
 xen/arch/x86/hvm/ioreq.c|   3 +-
 xen/arch/x86/hvm/irq.c  |   3 -
 xen/arch/x86/hvm/vmx/vmcs.c |  35 +
 xen/arch/x86/hvm/vmx/vmx.c  |  12 +-
 xen/arch/x86/mm.c   |   2 +-
 xen/arch/x86/mm/p2m-pt.c|   2 +-
 xen/arch/x86/mm/p2m.c   |   6 +-
 xen/arch/x86/physdev.c  |   8 -
 xen/arch/x86/setup.c|   7 -
 xen/arch/x86/time.c |  27 
 xen/common/domain.c |   2 -
 xen/common/domctl.c |  10 --
 xen/common/kernel.c |   5 -
 xen/common/vm_event.c   |   8 +-
 xen/include/asm-x86/domain.h|   1 -
 xen/include/asm-x86/hvm/hvm.h   |   3 -
 xen/include/public/domctl.h |  14 +-
 xen/include/xen/sched.h |   9 +-
 55 files changed, 252 insertions(+), 911 deletions(-)
 delete mode 100644 docs/misc/pvh-readme.txt

diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
index 505c111..8e4eb97 100644
--- a/docs/man/xl.cfg.pod.5.in
+++ b/docs/man/xl.cfg.pod.5.in
@@ -1064,6 +1064,13 @@ FIFO-based event channel ABI support up to 131,071 event 
channels.
 Other guests are limited to 4095 (64-bit x86 and ARM) or 1023 (32-bit
 x86).
 
+=item B
+
+Selects whether to create a PVH guest. This requires providing a kernel
+on the config file or using a boot loader or firmware.
+
+Default is 0.
+
 =back
 
 =head2 Paravirtualised (PV) Guest Specific Options
@@ -1108,10 +1115,6 @@ if your particular guest kernel does not require this 
behaviour then
 it is safe to allow this to be enabled but you may wish to disable it
 anyway.
 
-=item B
-
-Selects whether to run this PV guest in an HVM container. Default is 0.
-
 =back
 
 =head2 Fully-virtualised (HVM) Guest Specific Options
@@ -1925,11 +1928,6 @@ This device-model is the default for Linux dom0.
 Use the device-model based upon the historical Xen fork of Qemu.
 This device-model is still the default for NetBSD dom0.
 
-=item B
-
-Don't use any device model. This requires a kernel capable of booting
-without emulated devices.
-
 =back
 
 It is recomme

[Xen-devel] [PATCH v3 3/4] x86: remove has_hvm_container_{domain/vcpu}

[Roger Pau Monne]

It is now useless since PVHv1 is removed and PVHv2 is a HVM domain from Xen's
point of view.

Signed-off-by: Roger Pau Monné 
Reviewed-by: Andrew Cooper 
Acked-by: Tim Deegan 
Reviewed-by: Kevin Tian 
Reviewed-by: Boris Ostrovsky 
Acked-by: George Dunlap 
---
Cc: Christoph Egger 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Boris Ostrovsky 
Cc: Suravee Suthikulpanit 
Cc: Jun Nakajima 
Cc: Kevin Tian 
Cc: Elena Ufimtseva 
Cc: George Dunlap 
Cc: Tim Deegan 
Cc: Konrad Rzeszutek Wilk 
---
 xen/arch/x86/cpu/mcheck/vmce.c  |  6 +++---
 xen/arch/x86/cpu/vpmu.c |  4 ++--
 xen/arch/x86/cpu/vpmu_amd.c | 12 ++--
 xen/arch/x86/cpu/vpmu_intel.c   | 31 +++
 xen/arch/x86/cpuid.c|  6 +++---
 xen/arch/x86/debug.c|  2 +-
 xen/arch/x86/domain.c   | 28 ++--
 xen/arch/x86/domain_build.c |  5 ++---
 xen/arch/x86/domctl.c   |  2 +-
 xen/arch/x86/hvm/dm.c   |  2 +-
 xen/arch/x86/hvm/hvm.c  |  6 +++---
 xen/arch/x86/hvm/irq.c  |  2 +-
 xen/arch/x86/hvm/mtrr.c |  2 +-
 xen/arch/x86/hvm/vmsi.c |  3 +--
 xen/arch/x86/hvm/vmx/vmcs.c |  4 ++--
 xen/arch/x86/hvm/vmx/vmx.c  |  4 ++--
 xen/arch/x86/mm.c   |  4 ++--
 xen/arch/x86/mm/paging.c|  2 +-
 xen/arch/x86/mm/shadow/common.c |  9 -
 xen/arch/x86/setup.c|  2 +-
 xen/arch/x86/time.c | 11 +--
 xen/arch/x86/traps.c|  4 ++--
 xen/arch/x86/x86_64/traps.c |  4 ++--
 xen/drivers/passthrough/x86/iommu.c |  2 +-
 xen/include/asm-x86/domain.h|  2 +-
 xen/include/asm-x86/event.h |  2 +-
 xen/include/asm-x86/guest_access.h  | 12 ++--
 xen/include/asm-x86/hvm/hvm.h   |  2 +-
 xen/include/xen/sched.h |  2 --
 xen/include/xen/tmem_xen.h  |  5 ++---
 30 files changed, 87 insertions(+), 95 deletions(-)

diff --git a/xen/arch/x86/cpu/mcheck/vmce.c b/xen/arch/x86/cpu/mcheck/vmce.c
index 8b727b4..6fb7833 100644
--- a/xen/arch/x86/cpu/mcheck/vmce.c
+++ b/xen/arch/x86/cpu/mcheck/vmce.c
@@ -82,7 +82,7 @@ int vmce_restore_vcpu(struct vcpu *v, const struct 
hvm_vmce_vcpu *ctxt)
 {
 dprintk(XENLOG_G_ERR, "%s restore: unsupported MCA capabilities"
 " %#" PRIx64 " for %pv (supported: %#Lx)\n",
-has_hvm_container_vcpu(v) ? "HVM" : "PV", ctxt->caps,
+is_hvm_vcpu(v) ? "HVM" : "PV", ctxt->caps,
 v, guest_mcg_cap & ~MCG_CAP_COUNT);
 return -EPERM;
 }
@@ -364,7 +364,7 @@ int inject_vmce(struct domain *d, int vcpu)
 if ( !v->is_initialised )
 continue;
 
-if ( (has_hvm_container_domain(d) ||
+if ( (is_hvm_domain(d) ||
   guest_has_trap_callback(d, v->vcpu_id, TRAP_machine_check)) &&
  !test_and_set_bool(v->mce_pending) )
 {
@@ -444,7 +444,7 @@ int unmmap_broken_page(struct domain *d, mfn_t mfn, 
unsigned long gfn)
 if ( !mfn_valid(mfn) )
 return -EINVAL;
 
-if ( !has_hvm_container_domain(d) || !paging_mode_hap(d) )
+if ( !is_hvm_domain(d) || !paging_mode_hap(d) )
 return -EOPNOTSUPP;
 
 rc = -1;
diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
index a1e9f00..03401fd 100644
--- a/xen/arch/x86/cpu/vpmu.c
+++ b/xen/arch/x86/cpu/vpmu.c
@@ -237,7 +237,7 @@ void vpmu_do_interrupt(struct cpu_user_regs *regs)
 vpmu->arch_vpmu_ops->arch_vpmu_save(sampling, 1);
 vpmu_reset(vpmu, VPMU_CONTEXT_SAVE | VPMU_CONTEXT_LOADED);
 
-if ( has_hvm_container_vcpu(sampled) )
+if ( is_hvm_vcpu(sampled) )
 *flags = 0;
 else
 *flags = PMU_SAMPLE_PV;
@@ -288,7 +288,7 @@ void vpmu_do_interrupt(struct cpu_user_regs *regs)
 r->sp = cur_regs->rsp;
 r->flags = cur_regs->rflags;
 
-if ( !has_hvm_container_vcpu(sampled) )
+if ( !is_hvm_vcpu(sampled) )
 {
 r->ss = cur_regs->ss;
 r->cs = cur_regs->cs;
diff --git a/xen/arch/x86/cpu/vpmu_amd.c b/xen/arch/x86/cpu/vpmu_amd.c
index e0acbf4..b3c3697 100644
--- a/xen/arch/x86/cpu/vpmu_amd.c
+++ b/xen/arch/x86/cpu/vpmu_amd.c
@@ -305,8 +305,8 @@ static int amd_vpmu_save(struct vcpu *v,  bool_t to_guest)
 
 context_save(v);
 
-if ( !vpmu_is_set(vpmu, VPMU_RUNNING) &&
- has_hvm_container_vcpu(v) && is_msr_bitmap_on(vpmu) )
+if ( !vpmu_is_set(vpmu, VPMU_RUNNING) && is_hvm_vcpu(v) &&
+ is_msr_bitmap_on(vpmu) )
 amd_vpmu_unset_msr_bitmap(v);
 
 if ( to_guest )
@@ -367,7 +367,7 @@ static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t 
msr_content,
 return -EINVAL;
 
 /* For all counters, enable guest only mode for HVM guest */
-if ( has_hvm_container_vcpu(v) && (type == MSR_TYPE_CTRL) &&
+if ( is_hvm_vcpu(v) && (type == MSR_TYPE_CTRL) &&
 

Re: [Xen-devel] [RFC PATCH v1 16/21] ARM: NUMA: Extract proximity from SRAT table

[Vijay Kilari]

On Thu, Mar 2, 2017 at 10:51 PM, Julien Grall  wrote:
> (+ Jan as ACPI maintainer)
>
> Hello Vijay,
>
> On 09/02/17 15:57, vijay.kil...@gmail.com wrote:
>>
>> From: Vijaya Kumar K 
>>
>> Register SRAT entry handler for type
>> ACPI_SRAT_TYPE_GICC_AFFINITY to parse SRAT table
>> and extract proximity for all CPU IDs.
>>
>> Signed-off-by: Vijaya Kumar 
>> ---
>>  xen/arch/arm/acpi_numa.c  | 55
>> +++
>>  xen/drivers/acpi/numa.c   | 37 +++
>>  xen/drivers/acpi/osl.c|  2 ++
>>  xen/include/acpi/actbl1.h | 17 ++-
>>  xen/include/xen/acpi.h| 39 +
>>  5 files changed, 149 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/acpi_numa.c b/xen/arch/arm/acpi_numa.c
>> index 3ee87f2..f659275 100644
>> --- a/xen/arch/arm/acpi_numa.c
>> +++ b/xen/arch/arm/acpi_numa.c
>> @@ -105,6 +105,61 @@ static int __init acpi_parse_madt_handler(struct
>> acpi_subtable_header *header,
>>  return 0;
>>  }
>>
>> +/* Callback for Proximity Domain -> ACPI processor UID mapping */
>> +void __init acpi_numa_gicc_affinity_init(const struct
>> acpi_srat_gicc_affinity *pa)
>> +{
>> +int pxm, node;
>> +u64 mpidr = 0;
>
>
> mpidr does not need to be set to 0.
>
>> +static u32 cpus_in_srat;
>> +
>> +if ( srat_disabled() )
>> +return;
>> +
>> +if ( pa->header.length < sizeof(struct acpi_srat_gicc_affinity) )
>> +{
>> +printk(XENLOG_WARNING "SRAT: Invalid SRAT header length: %d\n",
>> +   pa->header.length);
>> +bad_srat();
>> +return;
>> +}
>> +
>> +if ( !(pa->flags & ACPI_SRAT_GICC_ENABLED) )
>> +return;
>> +
>> +if ( cpus_in_srat >= NR_CPUS )
>> +{
>> +printk(XENLOG_WARNING
>
>
> This should be XENLOG_ERROR.
OK
>
>> +   "SRAT: cpu_to_node_map[%d] is too small to fit all
>> cpus\n",
>> +   NR_CPUS);
>> +return;
>> +}
>> +
>> +pxm = pa->proximity_domain;
>> +node = setup_node(pxm);
>> +if ( node == NUMA_NO_NODE || node >= MAX_NUMNODES )
>
>
> Looking at the implementation of setup_node, node will either be equal to
> NUMA_NO_NODE or valid. It is not possible to have node >= MAX_NUMNODES.
ok
>
>> +{
>> +printk(XENLOG_WARNING "SRAT: Too many proximity domains %d\n",
>> pxm);
>
>
> setup_node is already printing an error if we have too many proximity
> domains. So no need to duplicate twice.
ok
>
>> +bad_srat();
>> +return;
>> +}
>> +
>> +mpidr = acpi_get_cpu_mpidr(pa->acpi_processor_uid);
>> +if ( mpidr == MPIDR_INVALID )
>> +{
>> +printk(XENLOG_WARNING
>
>
> s/XENLOG_WARNING/XENLOG_ERROR/
>
>> +   "SRAT: PXM %d with ACPI ID %d has no valid MPIDR in
>> MADT\n",
>> +   pxm, pa->acpi_processor_uid);
>> +bad_srat();
>> +return;
>> +}
>> +
>> +node_set(node, numa_nodes_parsed);
>> +cpus_in_srat++;
>> +acpi_numa = 1;
>> +printk(XENLOG_INFO "SRAT: PXM %d -> MPIDR 0x%lx -> Node %d\n",
>> +   pxm, mpidr, node);
>> +}
>> +
>>  void __init acpi_map_uid_to_mpidr(void)
>>  {
>>  int i;
>> diff --git a/xen/drivers/acpi/numa.c b/xen/drivers/acpi/numa.c
>> index 50bf9f8..ce22e88 100644
>> --- a/xen/drivers/acpi/numa.c
>> +++ b/xen/drivers/acpi/numa.c
>> @@ -25,9 +25,11 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>
>
> Why do you need to inlucde xen/mm.h and ...
>
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>
>
> asm/mm.h?

I remember when CONFIG_ACPI +CONFIG_NUMA is enabled
there is compilation error.

>
>
>>
>>  #define ACPI_NUMA  0x8000
>>  #define _COMPONENT ACPI_NUMA
>> @@ -105,6 +107,21 @@ void __init acpi_table_print_srat_entry(struct
>> acpi_subtable_header * header)
>> }
>>  #endif /* ACPI_DEBUG_OUTPUT */
>> break;
>> +   case ACPI_SRAT_TYPE_GICC_AFFINITY:
>> +#ifdef ACPI_DEBUG_OUTPUT
>> +   {
>> +   struct acpi_srat_gicc_affinity *p =
>> +   (struct acpi_srat_gicc_affinity *)header;
>> +   ACPI_DEBUG_PRINT((ACPI_DB_INFO,
>> + "SRAT Processor (acpi
>> id[0x%04x]) in"
>> + " proximity domain %d %s\n",
>> + p->acpi_processor_uid,
>> + p->proximity_domain,
>> + (p->flags &
>> ACPI_SRAT_GICC_ENABLED) ?
>> + "enabled" : "disabled");
>> +   }
>> +#endif /* ACPI_DEBUG_OUTPUT */
>> +   break;
>> default:
>> printk(KERN_WARNING PREFIX
>>"Found unsupported SRAT entry (type = %#x)\n",
>> @@ -185,6 +202,24 @@ int __init acpi_parse_srat(struct acpi_table_header
>> *

Re: [Xen-devel] [RFC PATCH v1 18/21] ARM: NUMA: update node_distance with ACPI support

[Vijay Kilari]

On Thu, Mar 2, 2017 at 10:54 PM, Julien Grall  wrote:
> Hello Vijay,
>
>
> On 09/02/17 15:57, vijay.kil...@gmail.com wrote:
>>
>> From: Vijaya Kumar K 
>>
>> Update node_distance() function to handle
>> ACPI SLIT table information.
>>
>> Signed-off-by: Vijaya Kumar 
>> ---
>>  xen/arch/arm/numa.c | 20 +++-
>>  1 file changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/numa.c b/xen/arch/arm/numa.c
>> index 5c49347..50c3dea 100644
>> --- a/xen/arch/arm/numa.c
>> +++ b/xen/arch/arm/numa.c
>> @@ -23,6 +23,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -35,6 +36,7 @@ extern struct node nodes[MAX_NUMNODES] __initdata;
>>  extern int num_node_memblks;
>>  extern struct node node_memblk_range[NR_NODE_MEMBLKS];
>>  extern nodeid_t memblk_nodeid[NR_NODE_MEMBLKS];
>> +extern struct acpi_table_slit *__read_mostly acpi_slit;
>>
>>  void __init numa_set_cpu_node(int cpu, unsigned long hwid)
>>  {
>> @@ -50,9 +52,24 @@ void __init numa_set_cpu_node(int cpu, unsigned long
>> hwid)
>>
>>  u8 __node_distance(nodeid_t a, nodeid_t b)
>>  {
>> -if ( !node_distance )
>> +unsigned index;
>> +u8 slit_val;
>> +
>> +if ( !node_distance && !acpi_slit )
>>  return a == b ? 10 : 20;
>>
>> +if ( acpi_slit )
>> +{
>> +index = acpi_slit->locality_count * node_to_pxm(a);
>> +slit_val = acpi_slit->entry[index + node_to_pxm(b)];
>> +
>> +/* ACPI defines 0xff as an unreachable node and 0-9 are undefined
>> */
>> +if ( (slit_val == 0xff) || (slit_val <= 9) )
>> +return NUMA_NO_DISTANCE;
>> +else
>> +return slit_val;
>> +}
>> +
>
>
> arm/numa.c is the generic code and should not contain any ACPI specific
> code.
Agreed.
>
> But as I said, the way to get the distance on ACPI is the same on x86 and
> ARM.
>
> So I would introduce __node_distance callback that will be setup at
> boot-time to either point to the ACPI version or DT version.

Instead of callback, Just call acpi's node_distance function if acpi is enabled
or else dt based.

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

Re: [Xen-devel] [RFC PATCH v1 19/21] ARM: NUMA: Initialize ACPI NUMA

[Vijay Kilari]

On Thu, Mar 2, 2017 at 10:55 PM, Julien Grall  wrote:
> Hello Vijay,
>
>
> On 09/02/17 15:57, vijay.kil...@gmail.com wrote:
>>
>> From: Vijaya Kumar K 
>>
>> Call ACPI NUMA initialization under CONFIG_ACPI_NUMA.
>>
>> Signed-off-by: Vijaya Kumar 
>> ---
>>  xen/arch/arm/numa.c | 12 +++-
>>  xen/common/numa.c   |  6 ++
>>  2 files changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/numa.c b/xen/arch/arm/numa.c
>> index 50c3dea..1d6e16c 100644
>> --- a/xen/arch/arm/numa.c
>> +++ b/xen/arch/arm/numa.c
>> @@ -204,7 +204,17 @@ int __init numa_init(void)
>>  for ( i = 0; i < MAX_NUMNODES * 2; i++ )
>>  _node_distance[i] = 0;
>>
>> -ret = dt_numa_init();
>> +#ifdef CONFIG_ACPI_NUMA
>> +if ( !acpi_disabled )
>> +{
>> +acpi_map_uid_to_mpidr();
>> +ret = acpi_numa_init();
>> +if ( ret || srat_disabled() )
>> +goto no_numa;
>> +}
>> +else
>> +#endif
>
>
> We should really have only on call to ACPI in the generic code. Please move
> all of this in a function.
OK
>
>
>> +ret = dt_numa_init();
>>
>>  if ( !ret )
>>  ret = numa_initmem_init();
>> diff --git a/xen/common/numa.c b/xen/common/numa.c
>> index 2f5266a..4c67d38 100644
>> --- a/xen/common/numa.c
>> +++ b/xen/common/numa.c
>> @@ -30,6 +30,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>
>>  static int numa_setup(char *s);
>> @@ -282,6 +283,11 @@ static __init int numa_setup(char *opt)
>>  numa_off = 1;
>>  if ( !strncmp(opt,"on",2) )
>>  numa_off = 0;
>> +if ( !strncmp(opt,"noacpi",6) )
>> +{
>> +numa_off = 0;
>> +acpi_numa = -1;
>> +}
>>
>>  return arch_numa_setup(opt);
>>  }
>>
>
> --
> Julien Grall

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

Re: [Xen-devel] [PATCH 2/5] x86/vioapic: allow the vIO APIC to have a variable number of pins

[Roger Pau Monne]

On Fri, Mar 03, 2017 at 04:56:20AM -0700, Jan Beulich wrote:
> >>> On 23.02.17 at 12:52,  wrote:
> > @@ -424,40 +424,141 @@ void vioapic_update_EOI(struct domain *d, u8 vector)
> >  spin_unlock(&d->arch.hvm_domain.irq_lock);
> >  }
> >  
> > -static int ioapic_save(struct domain *d, hvm_domain_context_t *h)
> > +#define VIOAPIC_SAVE_CONST offsetof(struct hvm_hw_vioapic, redirtbl)
> 
> What is this redirtbl field, which oddly enough is a pointer (not allowed
> in the public interface) good for? I can't seem to find any use of it
> other than as marker (for use with offsetof()). With all the
> complications here and below plus the fixed number of entries
> remaining to be fixed for actual migration purposes I wonder if you
> wouldn't be better off by keeping the structure mostly as is, simply
> converting the redirtbl[] field to a union (guarded by a __XEN__
> conditional) containing both a fixed size array and a variable size
> one (or to be precise, a zero size one, as a variable size one is not
> allowed there).
> 
> Another alternative would be to introduce a Xen internal sibling
> struct with a variable size array, and with all fields properly build-
> time-asserted to be identical between both.
> 
> I'll skip the rest of the patch assuming much of it could be dropped
> this way.

That was indeed my first approach, but later patches introduce an array of
hvm_hw_vioapic structs (in order to support multiple IO APICs per domain), and
this doesn't work if the struct itself contains a variable size array. I can
encapsulate this inside of a hvm_vioapic struct, like:

struct hvm_vioapic {
struct hvm_hw_ioapic *vioapic;
}

And make and array of hvm_vioapics instead, but that seemed more cumbersome.

I know this is all quite painful, but I wasn't able to sport a better solution.

Thanks, Roger.

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

Re: [Xen-devel] [PATCH 2/5] x86/vioapic: allow the vIO APIC to have a variable number of pins

[Jan Beulich]

>>> On 03.03.17 at 13:53,  wrote:
> On Fri, Mar 03, 2017 at 04:56:20AM -0700, Jan Beulich wrote:
>> >>> On 23.02.17 at 12:52,  wrote:
>> > @@ -424,40 +424,141 @@ void vioapic_update_EOI(struct domain *d, u8 vector)
>> >  spin_unlock(&d->arch.hvm_domain.irq_lock);
>> >  }
>> >  
>> > -static int ioapic_save(struct domain *d, hvm_domain_context_t *h)
>> > +#define VIOAPIC_SAVE_CONST offsetof(struct hvm_hw_vioapic, redirtbl)
>> 
>> What is this redirtbl field, which oddly enough is a pointer (not allowed
>> in the public interface) good for? I can't seem to find any use of it
>> other than as marker (for use with offsetof()). With all the
>> complications here and below plus the fixed number of entries
>> remaining to be fixed for actual migration purposes I wonder if you
>> wouldn't be better off by keeping the structure mostly as is, simply
>> converting the redirtbl[] field to a union (guarded by a __XEN__
>> conditional) containing both a fixed size array and a variable size
>> one (or to be precise, a zero size one, as a variable size one is not
>> allowed there).
>> 
>> Another alternative would be to introduce a Xen internal sibling
>> struct with a variable size array, and with all fields properly build-
>> time-asserted to be identical between both.
>> 
>> I'll skip the rest of the patch assuming much of it could be dropped
>> this way.
> 
> That was indeed my first approach, but later patches introduce an array of
> hvm_hw_vioapic structs (in order to support multiple IO APICs per domain), and
> this doesn't work if the struct itself contains a variable size array.

By what are you indexing that array? How about using a linked list
instead (or see below)? There won't normally be many entries, so
traversal is not an issue.

> I can
> encapsulate this inside of a hvm_vioapic struct, like:
> 
> struct hvm_vioapic {
> struct hvm_hw_ioapic *vioapic;
> }
> 
> And make and array of hvm_vioapics instead, but that seemed more cumbersome.

Well, you don't need a structure here at all. Just have an array
of pointers. In any event I think the respective public interface
change should be avoided here if at all possible.

Jan


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

Re: [Xen-devel] [PATCH v2] x86/kconfig: Introduce CONFIG_PV and CONFIG_HVM

[Wei Liu]

On Fri, Mar 03, 2017 at 11:22:06AM +, Andrew Cooper wrote:
> Making PV and HVM guests individually compilable is useful as a reduction in
> hypervisor size, and as an aid to enforcing clean API boundaries.
> 
> Introduce CONFIG_PV and CONFIG_HVM, although there is a lot of work to do
> until either can actually be disabled.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH RFC 3/5] x86: split PV dom0 builder to domain_build_pv.c

[Wei Liu]

On Fri, Mar 03, 2017 at 04:15:19AM -0700, Jan Beulich wrote:
> >>> On 03.03.17 at 12:06,  wrote:
> > On Fri, Mar 03, 2017 at 09:41:09AM +, Wei Liu wrote:
> >> Long term we want to be able to disentangle PV and HVM code. Move the PV
> >> domain builder to a dedicated file.
> >> 
> >> This in turn requires exposing a few functions and variables via
> >> a new header domain_build.h.
> > 
> > I would add: "No functional change introduced", to make it clearer that it's
> > mostly just code movement.
> > 
> >> Signed-off-by: Wei Liu 
> >> ---
> >>  xen/arch/x86/Makefile  |   1 +
> >>  xen/arch/x86/domain_build.c| 894 
> >> +---
> >>  xen/arch/x86/domain_build.h|  32 ++
> > 
> > IMHO I would place this in include/asm-x86/, there are really no headers in
> > arch/x86/.
> 
> That's the wrong criteria: If all consumers of a header are in the same
> directory, putting it there is fine (even if it's the first one). In fact in
> such a case I consider it worse to put it in asm-x86/, as that more
> widely exposes such a supposedly limited scope header. And along

My thought as well.

> those lines I actually have plans to move some of the stuff currently
> under include/asm-x86/ into (sub-trees of) arch/x86/.
> 
> However, with ...
> 
> >> diff --git a/xen/arch/x86/domain_build_pv.c 
> >> b/xen/arch/x86/domain_build_pv.c
> > 
> > I would place this in arch/xen/x86/pv/domain_build.c
> 
> ... this the case becomes less clear: Personally I'm not a fan of
> #include "../file.h", so I wouldn't be certain which of the two is
> the less undesirable place for the header.
> 

But in this case I would prefer to move the to xen/include/xen and named
in dom0_build.h.

Wei.

> Jan
> 

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

Re: [Xen-devel] [PATCH RFC 4/5] x86: split PVH dom0 builder to domain_build_pv.c

[Wei Liu]

On Fri, Mar 03, 2017 at 11:16:39AM +, Roger Pau Monné wrote:
> On Fri, Mar 03, 2017 at 09:41:10AM +, Wei Liu wrote:
> > Long term we want to be able to disentangle PV and HVM code. Move the
> > PVH domain builder to a dedicated file. It will later depends on
> > CONFIG_HVM or CONFIG_PVH.
> > 
> > This in turn requires exposing a few functions and variables via
> > domain_build.h.
> 
> Again I would add something along the lines: "No functional changes expected".
> 
> > Signed-off-by: Wei Liu 
> > ---
> >  xen/arch/x86/Makefile   |1 +
> >  xen/arch/x86/domain_build.c | 1068 
> > +
> >  xen/arch/x86/domain_build.h |8 +
> >  xen/arch/x86/domain_build_pvh.c | 1097 
> > +++
> 
> I would prefer that to be in arch/x86/hvm/.
> 
> > diff --git a/xen/arch/x86/domain_build_pvh.c 
> > b/xen/arch/x86/domain_build_pvh.c
> > new file mode 100644
> > index 00..6cd3ff2ef2
> > --- /dev/null
> > +++ b/xen/arch/x86/domain_build_pvh.c
> > @@ -0,0 +1,1097 @@
> > +/**
> > + * domain_build_pvh.c
> > + */
> 
> Since you are already touching this, and it's mostly my code, I would rather
> prefer that you add the following header:
> 
> /**
>  * arch/x86/hvm/domain_build.c
>  *
>  * PVH domain builder
>  *
>  * This program is free software; you can redistribute it and/or modify
>  * it under the terms of the GNU General Public License as published by
>  * the Free Software Foundation; either version 2 of the License, or
>  * (at your option) any later version.

I suggest we use the inbound licence listed in CONTRIBUTING. I think it
is GPL v2 only, without the any later version part.

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

Re: [Xen-devel] [PATCH 1/6] xen: credit1: simplify csched_runq_steal() a little bit.

[Dario Faggioli]

On Fri, 2017-03-03 at 09:35 +, anshul makkar wrote:
> On 02/03/17 10:38, Dario Faggioli wrote:
> > --- a/xen/common/sched_credit.c
> > +++ b/xen/common/sched_credit.c
> > @@ -1593,64 +1593,65 @@ static struct csched_vcpu *
> >   csched_runq_steal(int peer_cpu, int cpu, int pri, int
> > balance_step)
> >   {
> >   const struct csched_pcpu * const peer_pcpu =
> > CSCHED_PCPU(peer_cpu);
> > -const struct vcpu * const peer_vcpu = curr_on_cpu(peer_cpu);
> >   struct csched_vcpu *speer;
> >   struct list_head *iter;
> >   struct vcpu *vc;
> >   
> > +ASSERT(peer_pcpu != NULL);
> > +
> >   /*
> >    * Don't steal from an idle CPU's runq because it's about to
> >    * pick up work from it itself.
> >    */
> > -if ( peer_pcpu != NULL && !is_idle_vcpu(peer_vcpu) )
> > +if ( unlikely(is_idle_vcpu(curr_on_cpu(peer_cpu))) )
> > +goto out;
> We can just use if (!is_idle_vcpu(peer_vcpu)). Why to replace it
> with 
> some code that introduces an unnecessary branch statement.
>
Mmm... I don't think I understand what this means.

> > +/*
> > + * If the vcpu has no useful soft affinity, skip this
> > vcpu.
> > + * In fact, what we want is to check if we have any "soft-
> > affine
> > + * work" to steal, before starting to look at "hard-affine 
> > work".
> > + *
> > + * Notice that, if not even one vCPU on this runq has a
> > useful
> > + * soft affinity, we could have avoid considering this
> > runq for
> > + * a soft balancing step in the first place. This, for
> > instance,
> > + * can be implemented by taking note of on what runq there
> > are
> > + * vCPUs with useful soft affinities in some sort of
> > bitmap
> > + * or counter.
> > + */
>
> Isn't it a better approach that now as we have came across a vcpu
> which 
> doesn't have a desired soft affinity but is a potential candidate
> for 
> migration, so instead of just forgetting it,  lets save the
> information 
> for such vcpus in some data structure in some order so that we don't 
> have to scan them again if we don't find anything useful in the
> present run.
>
So, AFAIUI, you're suggesting something like this:
 1. for each vcpu in the runqueue, we check soft-affinity. If it 
    matches, we're done;
 2. if it does not match, we check hard-affinity. If it matches, we 
    save that vcpu somewhere. We only need to save one vcpu, the first 
    one that we find to have matching hard-affinity;
 3. if we don't find any vcpu with matching soft affinity, we steal 
    the one we've saved.

Is this correct? If yes, if there's not anything I'm overlooking, I
think it could work.

It's a separate patch, of course. I can try putting that together,
unless of course you want to give this a go yourself. :-)

Thanks and 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 RFC 0/5] Refactor x86 domain builder

[Wei Liu]

On Fri, Mar 03, 2017 at 04:06:00AM -0700, Jan Beulich wrote:
> >>> On 03.03.17 at 11:56,  wrote:
> > On 03/03/17 09:41, Wei Liu wrote:
> > Jan: Would you be happy accepting my CONFIG_{PV,HVM} patch in its silent
> > form for now?  If so, I will respin it.
> 
> I think so, yes.
> 
> >> This series is built on top of Roger's PVHv1 removal series.
> >>
> >> Wei Liu (5):
> >>   xen: move round_pg{up,down} to pfn.h
> >>   xen: include xen/types.h in domain.h
> >>   x86: split PV dom0 builder to domain_build_pv.c
> >>   x86: split PVH dom0 builder to domain_build_pv.c
> >>   x86: clean up header files in domain_build.c
> > 
> > I'd argue that these should be named differently.  They are specifically
> > hwdom_build.c, distinct from general domain building operations.
> 
> hwdom_build.c is wrong too - this is strictly Dom0, i.e. it ought
> to be dom0_build.c.

Yes, I agree.

We shall have dom0_build.c, dom0_build_pv.c and dom0_build_pvh.c in v2.

Wei.

> 
> Jan
> 

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

[Xen-devel] [xen-unstable-smoke test] 106402: tolerable trouble: broken/fail/pass - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 build-arm64   5 xen-buildfail   never pass
 build-arm64-pvops 5 kernel-build fail   never pass
 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  b151125b4d89d7ec139ac34470e3c709fb4b1b4d
baseline version:
 xen  7bdb974a82c1631bfc7451b9dd9756858617aef4

Last test of basis   106380  2017-03-02 20:02:17 Z0 days
Testing same since   106402  2017-03-03 12:02:59 Z0 days1 attempts


People who touched revisions under test:
  Chao Gao 
  Feng Wu 
  Jan Beulich 
  Kevin Tian 
  Quan Xu 
  Sergey Dyasli 

jobs:
 build-amd64  pass
 build-arm64  fail
 build-armhf  pass
 build-amd64-libvirt  pass
 build-arm64-pvopsfail
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  broken  
 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=b151125b4d89d7ec139ac34470e3c709fb4b1b4d
+ . ./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 
b151125b4d89d7ec139ac34470e3c709fb4b1b4d
+ branch=xen-unstable-smoke
+ revision=b151125b4d89d7ec139ac34470e3c709fb4b1b4d
+ . ./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
+ '[' xb151125b4d89d7ec139ac34470e3c709fb4b1b4d = 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.o

Re: [Xen-devel] [RFC PATCH v1 16/21] ARM: NUMA: Extract proximity from SRAT table

[Julien Grall]

Hello Vijay,

On 03/03/17 12:39, Vijay Kilari wrote:

On Thu, Mar 2, 2017 at 10:51 PM, Julien Grall  wrote:

diff --git a/xen/drivers/acpi/numa.c b/xen/drivers/acpi/numa.c
index 50bf9f8..ce22e88 100644
--- a/xen/drivers/acpi/numa.c
+++ b/xen/drivers/acpi/numa.c
@@ -25,9 +25,11 @@
 #include 
 #include 
 #include 
+#include 



Why do you need to inlucde xen/mm.h and ...


 #include 
 #include 
 #include 
+#include 



asm/mm.h?


I remember when CONFIG_ACPI +CONFIG_NUMA is enabled
there is compilation error.


Regarding asm/mm.h, it is already included by xen/mm.h. So no point to 
add it.


Now regarding xen/mm.h, just saying there is a compilation error is not 
helpful. Can you expand why it is needed?


[...]


This is quite disgusting. We should avoid any #ifdef CONFIG_{X86,ARM} in
common header.

Also, x2apic and gicc are respectively x86-specific and arm-specific. So I
think we should move the parsing in a separate arch-depend function to avoid
those ifdery.

By that I mean having a function acpi_table_arch_parse_srat that will parse
x2apci on x86 and gicc on ARM. Jan, what do you think?


Linux also follows similar approach
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/linux/acpi.h?id=refs/tags/v4.10#n265


So? What are you trying to prove?

The linux version is much readable than yours. Anyway, we should limit 
CONFIG_{X86,ARM} ifdefery in common code.


Currently, I see no point to have those ifdefery when it is possible to 
add an arch-depend function.


Regards,

--
Julien Grall

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

Re: [Xen-devel] [RFC PATCH v1 18/21] ARM: NUMA: update node_distance with ACPI support

[Julien Grall]

Hello Vijay,

On 03/03/17 12:43, Vijay Kilari wrote:

On Thu, Mar 2, 2017 at 10:54 PM, Julien Grall  wrote:

Hello Vijay,


On 09/02/17 15:57, vijay.kil...@gmail.com wrote:


From: Vijaya Kumar K 

Update node_distance() function to handle
ACPI SLIT table information.

Signed-off-by: Vijaya Kumar 
---
 xen/arch/arm/numa.c | 20 +++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/numa.c b/xen/arch/arm/numa.c
index 5c49347..50c3dea 100644
--- a/xen/arch/arm/numa.c
+++ b/xen/arch/arm/numa.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -35,6 +36,7 @@ extern struct node nodes[MAX_NUMNODES] __initdata;
 extern int num_node_memblks;
 extern struct node node_memblk_range[NR_NODE_MEMBLKS];
 extern nodeid_t memblk_nodeid[NR_NODE_MEMBLKS];
+extern struct acpi_table_slit *__read_mostly acpi_slit;

 void __init numa_set_cpu_node(int cpu, unsigned long hwid)
 {
@@ -50,9 +52,24 @@ void __init numa_set_cpu_node(int cpu, unsigned long
hwid)

 u8 __node_distance(nodeid_t a, nodeid_t b)
 {
-if ( !node_distance )
+unsigned index;
+u8 slit_val;
+
+if ( !node_distance && !acpi_slit )
 return a == b ? 10 : 20;

+if ( acpi_slit )
+{
+index = acpi_slit->locality_count * node_to_pxm(a);
+slit_val = acpi_slit->entry[index + node_to_pxm(b)];
+
+/* ACPI defines 0xff as an unreachable node and 0-9 are undefined
*/
+if ( (slit_val == 0xff) || (slit_val <= 9) )
+return NUMA_NO_DISTANCE;
+else
+return slit_val;
+}
+



arm/numa.c is the generic code and should not contain any ACPI specific
code.

Agreed.


But as I said, the way to get the distance on ACPI is the same on x86 and
ARM.

So I would introduce __node_distance callback that will be setup at
boot-time to either point to the ACPI version or DT version.


Instead of callback, Just call acpi's node_distance function if acpi is enabled
or else dt based.


Why? I don't see any reason to want checking whether acpi is enabled 
everytime __node_distance is called. The value will never change at runtime.


Regards,

--
Julien Grall

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

Re: [Xen-devel] [RFC PATCH v1 16/21] ARM: NUMA: Extract proximity from SRAT table

[Vijay Kilari]

On Fri, Mar 3, 2017 at 7:14 PM, Julien Grall  wrote:
> Hello Vijay,
>
> On 03/03/17 12:39, Vijay Kilari wrote:
>>
>> On Thu, Mar 2, 2017 at 10:51 PM, Julien Grall 
>> wrote:

 diff --git a/xen/drivers/acpi/numa.c b/xen/drivers/acpi/numa.c
 index 50bf9f8..ce22e88 100644
 --- a/xen/drivers/acpi/numa.c
 +++ b/xen/drivers/acpi/numa.c
 @@ -25,9 +25,11 @@
  #include 
  #include 
  #include 
 +#include 
>>>
>>>
>>>
>>> Why do you need to inlucde xen/mm.h and ...
>>>
  #include 
  #include 
  #include 
 +#include 
>>>
>>>
>>>
>>> asm/mm.h?
>>
>>
>> I remember when CONFIG_ACPI +CONFIG_NUMA is enabled
>> there is compilation error.
>
>
> Regarding asm/mm.h, it is already included by xen/mm.h. So no point to add
> it.
>
> Now regarding xen/mm.h, just saying there is a compilation error is not
> helpful. Can you expand why it is needed?

I remember just adding xen/mm.h has not solved the problem. Anyway I
will check this
when I work for next revision.

>
> [...]
>
>>> This is quite disgusting. We should avoid any #ifdef CONFIG_{X86,ARM} in
>>> common header.
>>>
>>> Also, x2apic and gicc are respectively x86-specific and arm-specific. So
>>> I
>>> think we should move the parsing in a separate arch-depend function to
>>> avoid
>>> those ifdery.
>>>
>>> By that I mean having a function acpi_table_arch_parse_srat that will
>>> parse
>>> x2apci on x86 and gicc on ARM. Jan, what do you think?
>>
>>
>> Linux also follows similar approach
>>
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/linux/acpi.h?id=refs/tags/v4.10#n265
>
>
> So? What are you trying to prove?
>
> The linux version is much readable than yours. Anyway, we should limit
> CONFIG_{X86,ARM} ifdefery in common code.
>
> Currently, I see no point to have those ifdefery when it is possible to add
> an arch-depend function.

This is because in xen we have another level of config CONFIG_ACPI_NUMA.
I have plans to reuse cpu and memory part next revision.

Regards
Vijay

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

Re: [Xen-devel] [RFC PATCH v1 16/21] ARM: NUMA: Extract proximity from SRAT table

[Julien Grall]

On 03/03/17 13:50, Vijay Kilari wrote:

On Fri, Mar 3, 2017 at 7:14 PM, Julien Grall  wrote:

This is quite disgusting. We should avoid any #ifdef CONFIG_{X86,ARM} in
common header.

Also, x2apic and gicc are respectively x86-specific and arm-specific. So
I
think we should move the parsing in a separate arch-depend function to
avoid
those ifdery.

By that I mean having a function acpi_table_arch_parse_srat that will
parse
x2apci on x86 and gicc on ARM. Jan, what do you think?



Linux also follows similar approach

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/linux/acpi.h?id=refs/tags/v4.10#n265



So? What are you trying to prove?

The linux version is much readable than yours. Anyway, we should limit
CONFIG_{X86,ARM} ifdefery in common code.

Currently, I see no point to have those ifdefery when it is possible to add
an arch-depend function.


This is because in xen we have another level of config CONFIG_ACPI_NUMA.
I have plans to reuse cpu and memory part next revision.


This does not explain why you want to do like Linux.

--
Julien Grall

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

Re: [Xen-devel] [PATCH 2/6] xen: credit: (micro) optimize csched_runq_steal().

[Dario Faggioli]

On Fri, 2017-03-03 at 09:48 +, anshul makkar wrote:
> On 02/03/17 10:38, Dario Faggioli wrote:
> > --- a/xen/common/sched_credit.c
> > +++ b/xen/common/sched_credit.c
> > @@ -708,12 +708,10 @@ static inline int
> >   __csched_vcpu_is_migrateable(struct vcpu *vc, int dest_cpu,
> > cpumask_t *mask)
> >   {
> >   /*
> > - * Don't pick up work that's in the peer's scheduling tail or
> > hot on
> > - * peer PCPU. Only pick up work that prefers and/or is allowed
> > to run
> > - * on our CPU.
> > + * Don't pick up work that's or hot on peer PCPU, or that
> > can't (or
> Not clear.
>
Well, there's actually a typo (redundant 'or'). Good catch. :-)
> > 
> > + * would prefer not to) run on cpu.
> >    */
> > -return !vc->is_running &&
> > -   !__csched_vcpu_is_cache_hot(vc) &&
> > +return !__csched_vcpu_is_cache_hot(vc) &&
> >  cpumask_test_cpu(dest_cpu, mask);
> !vc->is_running doesn't ease the complexity and doesn't save much on
> cpu 
> cycles. Infact, I think (!vc->is_running) makes the intention to
> check 
> for migration much more clear to understand.
> 
But the point is not saving the overhead of a !vc->is_running check
here, it is actually to pull it out from within this function and check
that before. And that's ok because the value won't change, and is good
thing because what we save is a call to

  __vcpu_has_soft_affinity()

and, potentially, to

  csched_balance_cpumask()

i.e., more specifically...
            *
> > @@ -1633,8 +1633,9 @@ csched_runq_steal(int peer_cpu, int cpu, int
> > pri, int balance_step)
> >    * vCPUs with useful soft affinities in some sort of
> > bitmap
> >    * or counter.
> >    */
> > -if ( balance_step == CSCHED_BALANCE_SOFT_AFFINITY
> > - && !__vcpu_has_soft_affinity(vc, vc-
> > >cpu_hard_affinity) )
> > +if ( vc->is_running ||
> > + (balance_step == CSCHED_BALANCE_SOFT_AFFINITY
> > +  && !__vcpu_has_soft_affinity(vc, vc-
> > >cpu_hard_affinity)) )
> >   continue;
> >   
> >   csched_balance_cpumask(vc, balance_step,
> > cpumask_scratch);
> > 
...these ones here.

I agree that the check was a good fit for that function, but --with the
updated comments-- I don't think it's too terrible to have it outside.

Or were you suggesting to have it in _both_ places? If that's the case,
no... I agree it's cheap, but that would look confusing to me (I
totally see myself, in 3 months, sending a patch to remove the
redundant is_running check! :-P).

Thanks and 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] xen-netback: fix race condition on XenBus disconnect

[Igor Druzhinin]

On 03/03/17 09:18, Paul Durrant wrote:
>> -Original Message-
>> From: Igor Druzhinin [mailto:igor.druzhi...@citrix.com]
>> Sent: 02 March 2017 22:57
>> To: net...@vger.kernel.org; xen-de...@lists.xenproject.org
>> Cc: Paul Durrant ; jgr...@suse.com; Wei Liu
>> ; Igor Druzhinin 
>> Subject: [PATCH] xen-netback: fix race condition on XenBus disconnect
>>
>> In some cases during XenBus disconnect event handling and subsequent
>> queue resource release there may be some TX handlers active on
>> other processors. Use RCU in order to synchronize with them.
>>
>> Signed-off-by: Igor Druzhinin 
>> ---
>>  drivers/net/xen-netback/interface.c | 13 -
>>  drivers/net/xen-netback/xenbus.c| 17 +++--
>>  2 files changed, 15 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-
>> netback/interface.c
>> index a2d32676..32e2cc6 100644
>> --- a/drivers/net/xen-netback/interface.c
>> +++ b/drivers/net/xen-netback/interface.c
>> @@ -164,7 +164,7 @@ static int xenvif_start_xmit(struct sk_buff *skb, struct
>> net_device *dev)
>>  {
>>  struct xenvif *vif = netdev_priv(dev);
>>  struct xenvif_queue *queue = NULL;
>> -unsigned int num_queues = vif->num_queues;
> 
> Do you not need an rcu_read_lock() around this and use of the num_queues 
> value (as you have below)?
> 
>> +unsigned int num_queues = rcu_dereference(vif)->num_queues;
>>  u16 index;
>>  struct xenvif_rx_cb *cb;
>>
>> @@ -221,18 +221,21 @@ static struct net_device_stats
>> *xenvif_get_stats(struct net_device *dev)
>>  {
>>  struct xenvif *vif = netdev_priv(dev);
>>  struct xenvif_queue *queue = NULL;
>> +unsigned int num_queues;
>>  u64 rx_bytes = 0;
>>  u64 rx_packets = 0;
>>  u64 tx_bytes = 0;
>>  u64 tx_packets = 0;
>>  unsigned int index;
>>
>> -spin_lock(&vif->lock);
>> -if (vif->queues == NULL)
>> +rcu_read_lock();
>> +
>> +num_queues = rcu_dereference(vif)->num_queues;
>> +if (num_queues < 1)
>>  goto out;
>>
>>  /* Aggregate tx and rx stats from each queue */
>> -for (index = 0; index < vif->num_queues; ++index) {
>> +for (index = 0; index < num_queues; ++index) {
>>  queue = &vif->queues[index];
>>  rx_bytes += queue->stats.rx_bytes;
>>  rx_packets += queue->stats.rx_packets;
>> @@ -241,7 +244,7 @@ static struct net_device_stats
>> *xenvif_get_stats(struct net_device *dev)
>>  }
>>
>>  out:
>> -spin_unlock(&vif->lock);
>> +rcu_read_unlock();
>>
>>  vif->dev->stats.rx_bytes = rx_bytes;
>>  vif->dev->stats.rx_packets = rx_packets;
>> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-
>> netback/xenbus.c
>> index d2d7cd9..76efb01 100644
>> --- a/drivers/net/xen-netback/xenbus.c
>> +++ b/drivers/net/xen-netback/xenbus.c
>> @@ -495,26 +495,23 @@ static void backend_disconnect(struct
>> backend_info *be)
>>  struct xenvif *vif = be->vif;
>>
>>  if (vif) {
>> +unsigned int num_queues = vif->num_queues;
>>  unsigned int queue_index;
>> -struct xenvif_queue *queues;
>>
>>  xen_unregister_watchers(vif);
>>  #ifdef CONFIG_DEBUG_FS
>>  xenvif_debugfs_delif(vif);
>>  #endif /* CONFIG_DEBUG_FS */
>>  xenvif_disconnect_data(vif);
>> -for (queue_index = 0;
>> - queue_index < vif->num_queues;
>> - ++queue_index)
>> -xenvif_deinit_queue(&vif->queues[queue_index]);
>>
>> -spin_lock(&vif->lock);
>> -queues = vif->queues;
>>  vif->num_queues = 0;
>> -vif->queues = NULL;
>> -spin_unlock(&vif->lock);
>> +synchronize_net();
> 
> So, num_queues is your RCU protected value, rather than the queues pointer, 
> in which case I think you probably need to change code such as
> 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/net/xen-netback/netback.c?id=refs/tags/v4.10#n216
> 
> to be gated on num_queues.
> 
> Also shouldn't xenvif_up(), xenvif_down() and xenvif_get_ethtool_stats() not 
> be using rcu_read_lock() and rcu_dereference() of num_queues as well?
> 
>   Paul
> 

xenvif_up() and xenvif_down() are called under rtnl_lock.
xenvif_get_ethtool_stats() is a good candidate. I'm not sure about
xenvif_fatal_tx_err. Is it actually called from already protected regions?

Igor

>>
>> -vfree(queues);
>> +for (queue_index = 0; queue_index < num_queues;
>> ++queue_index)
>> +xenvif_deinit_queue(&vif->queues[queue_index]);
>> +
>> +vfree(vif->queues);
>> +vif->queues = NULL;
>>
>>  xenvif_disconnect_ctrl(vif);
>>  }
>> --
>> 1.8.3.1
> 

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

Re: [Xen-devel] [PATCH] xen-netback: fix race condition on XenBus disconnect

[Igor Druzhinin]

On 03/03/17 09:18, Paul Durrant wrote:
>> -Original Message-
>> From: Igor Druzhinin [mailto:igor.druzhi...@citrix.com]
>> Sent: 02 March 2017 22:57
>> To: net...@vger.kernel.org; xen-de...@lists.xenproject.org
>> Cc: Paul Durrant ; jgr...@suse.com; Wei Liu
>> ; Igor Druzhinin 
>> Subject: [PATCH] xen-netback: fix race condition on XenBus disconnect
>>
>> In some cases during XenBus disconnect event handling and subsequent
>> queue resource release there may be some TX handlers active on
>> other processors. Use RCU in order to synchronize with them.
>>
>> Signed-off-by: Igor Druzhinin 
>> ---
>>  drivers/net/xen-netback/interface.c | 13 -
>>  drivers/net/xen-netback/xenbus.c| 17 +++--
>>  2 files changed, 15 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-
>> netback/interface.c
>> index a2d32676..32e2cc6 100644
>> --- a/drivers/net/xen-netback/interface.c
>> +++ b/drivers/net/xen-netback/interface.c
>> @@ -164,7 +164,7 @@ static int xenvif_start_xmit(struct sk_buff *skb, struct
>> net_device *dev)
>>  {
>>  struct xenvif *vif = netdev_priv(dev);
>>  struct xenvif_queue *queue = NULL;
>> -unsigned int num_queues = vif->num_queues;
> 
> Do you not need an rcu_read_lock() around this and use of the num_queues 
> value (as you have below)?

Huh, missed this one. Point is that xenvif_start_xmit is already in RCU
read section.

Igor

> 
>> +unsigned int num_queues = rcu_dereference(vif)->num_queues;
>>  u16 index;
>>  struct xenvif_rx_cb *cb;
>>
>> @@ -221,18 +221,21 @@ static struct net_device_stats
>> *xenvif_get_stats(struct net_device *dev)
>>  {
>>  struct xenvif *vif = netdev_priv(dev);
>>  struct xenvif_queue *queue = NULL;
>> +unsigned int num_queues;
>>  u64 rx_bytes = 0;
>>  u64 rx_packets = 0;
>>  u64 tx_bytes = 0;
>>  u64 tx_packets = 0;
>>  unsigned int index;
>>
>> -spin_lock(&vif->lock);
>> -if (vif->queues == NULL)
>> +rcu_read_lock();
>> +
>> +num_queues = rcu_dereference(vif)->num_queues;
>> +if (num_queues < 1)
>>  goto out;
>>
>>  /* Aggregate tx and rx stats from each queue */
>> -for (index = 0; index < vif->num_queues; ++index) {
>> +for (index = 0; index < num_queues; ++index) {
>>  queue = &vif->queues[index];
>>  rx_bytes += queue->stats.rx_bytes;
>>  rx_packets += queue->stats.rx_packets;
>> @@ -241,7 +244,7 @@ static struct net_device_stats
>> *xenvif_get_stats(struct net_device *dev)
>>  }
>>
>>  out:
>> -spin_unlock(&vif->lock);
>> +rcu_read_unlock();
>>
>>  vif->dev->stats.rx_bytes = rx_bytes;
>>  vif->dev->stats.rx_packets = rx_packets;
>> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-
>> netback/xenbus.c
>> index d2d7cd9..76efb01 100644
>> --- a/drivers/net/xen-netback/xenbus.c
>> +++ b/drivers/net/xen-netback/xenbus.c
>> @@ -495,26 +495,23 @@ static void backend_disconnect(struct
>> backend_info *be)
>>  struct xenvif *vif = be->vif;
>>
>>  if (vif) {
>> +unsigned int num_queues = vif->num_queues;
>>  unsigned int queue_index;
>> -struct xenvif_queue *queues;
>>
>>  xen_unregister_watchers(vif);
>>  #ifdef CONFIG_DEBUG_FS
>>  xenvif_debugfs_delif(vif);
>>  #endif /* CONFIG_DEBUG_FS */
>>  xenvif_disconnect_data(vif);
>> -for (queue_index = 0;
>> - queue_index < vif->num_queues;
>> - ++queue_index)
>> -xenvif_deinit_queue(&vif->queues[queue_index]);
>>
>> -spin_lock(&vif->lock);
>> -queues = vif->queues;
>>  vif->num_queues = 0;
>> -vif->queues = NULL;
>> -spin_unlock(&vif->lock);
>> +synchronize_net();
> 
> So, num_queues is your RCU protected value, rather than the queues pointer, 
> in which case I think you probably need to change code such as
> 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/net/xen-netback/netback.c?id=refs/tags/v4.10#n216
> 
> to be gated on num_queues.
> 
> Also shouldn't xenvif_up(), xenvif_down() and xenvif_get_ethtool_stats() not 
> be using rcu_read_lock() and rcu_dereference() of num_queues as well?
> 
>   Paul
> 
>>
>> -vfree(queues);
>> +for (queue_index = 0; queue_index < num_queues;
>> ++queue_index)
>> +xenvif_deinit_queue(&vif->queues[queue_index]);
>> +
>> +vfree(vif->queues);
>> +vif->queues = NULL;
>>
>>  xenvif_disconnect_ctrl(vif);
>>  }
>> --
>> 1.8.3.1
> 

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

Re: [Xen-devel] [PATCH] xen-netback: fix race condition on XenBus disconnect

[Paul Durrant]

> -Original Message-
> From: Igor Druzhinin
> Sent: 03 March 2017 13:56
> To: Paul Durrant ; net...@vger.kernel.org; xen-
> de...@lists.xenproject.org
> Cc: jgr...@suse.com; Wei Liu 
> Subject: Re: [PATCH] xen-netback: fix race condition on XenBus disconnect
> 
> On 03/03/17 09:18, Paul Durrant wrote:
> >> -Original Message-
> >> From: Igor Druzhinin [mailto:igor.druzhi...@citrix.com]
> >> Sent: 02 March 2017 22:57
> >> To: net...@vger.kernel.org; xen-de...@lists.xenproject.org
> >> Cc: Paul Durrant ; jgr...@suse.com; Wei Liu
> >> ; Igor Druzhinin 
> >> Subject: [PATCH] xen-netback: fix race condition on XenBus disconnect
> >>
> >> In some cases during XenBus disconnect event handling and subsequent
> >> queue resource release there may be some TX handlers active on
> >> other processors. Use RCU in order to synchronize with them.
> >>
> >> Signed-off-by: Igor Druzhinin 
> >> ---
> >>  drivers/net/xen-netback/interface.c | 13 -
> >>  drivers/net/xen-netback/xenbus.c| 17 +++--
> >>  2 files changed, 15 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-
> >> netback/interface.c
> >> index a2d32676..32e2cc6 100644
> >> --- a/drivers/net/xen-netback/interface.c
> >> +++ b/drivers/net/xen-netback/interface.c
> >> @@ -164,7 +164,7 @@ static int xenvif_start_xmit(struct sk_buff *skb,
> struct
> >> net_device *dev)
> >>  {
> >>struct xenvif *vif = netdev_priv(dev);
> >>struct xenvif_queue *queue = NULL;
> >> -  unsigned int num_queues = vif->num_queues;
> >
> > Do you not need an rcu_read_lock() around this and use of the
> num_queues value (as you have below)?
> 
> Huh, missed this one. Point is that xenvif_start_xmit is already in RCU
> read section.
> 

Ok. Probably worth a comment then since the rcu_deref looks wrong out on its 
own like that.

  Paul

> Igor
> 
> >
> >> +  unsigned int num_queues = rcu_dereference(vif)->num_queues;
> >>u16 index;
> >>struct xenvif_rx_cb *cb;
> >>
> >> @@ -221,18 +221,21 @@ static struct net_device_stats
> >> *xenvif_get_stats(struct net_device *dev)
> >>  {
> >>struct xenvif *vif = netdev_priv(dev);
> >>struct xenvif_queue *queue = NULL;
> >> +  unsigned int num_queues;
> >>u64 rx_bytes = 0;
> >>u64 rx_packets = 0;
> >>u64 tx_bytes = 0;
> >>u64 tx_packets = 0;
> >>unsigned int index;
> >>
> >> -  spin_lock(&vif->lock);
> >> -  if (vif->queues == NULL)
> >> +  rcu_read_lock();
> >> +
> >> +  num_queues = rcu_dereference(vif)->num_queues;
> >> +  if (num_queues < 1)
> >>goto out;
> >>
> >>/* Aggregate tx and rx stats from each queue */
> >> -  for (index = 0; index < vif->num_queues; ++index) {
> >> +  for (index = 0; index < num_queues; ++index) {
> >>queue = &vif->queues[index];
> >>rx_bytes += queue->stats.rx_bytes;
> >>rx_packets += queue->stats.rx_packets;
> >> @@ -241,7 +244,7 @@ static struct net_device_stats
> >> *xenvif_get_stats(struct net_device *dev)
> >>}
> >>
> >>  out:
> >> -  spin_unlock(&vif->lock);
> >> +  rcu_read_unlock();
> >>
> >>vif->dev->stats.rx_bytes = rx_bytes;
> >>vif->dev->stats.rx_packets = rx_packets;
> >> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-
> >> netback/xenbus.c
> >> index d2d7cd9..76efb01 100644
> >> --- a/drivers/net/xen-netback/xenbus.c
> >> +++ b/drivers/net/xen-netback/xenbus.c
> >> @@ -495,26 +495,23 @@ static void backend_disconnect(struct
> >> backend_info *be)
> >>struct xenvif *vif = be->vif;
> >>
> >>if (vif) {
> >> +  unsigned int num_queues = vif->num_queues;
> >>unsigned int queue_index;
> >> -  struct xenvif_queue *queues;
> >>
> >>xen_unregister_watchers(vif);
> >>  #ifdef CONFIG_DEBUG_FS
> >>xenvif_debugfs_delif(vif);
> >>  #endif /* CONFIG_DEBUG_FS */
> >>xenvif_disconnect_data(vif);
> >> -  for (queue_index = 0;
> >> -   queue_index < vif->num_queues;
> >> -   ++queue_index)
> >> -  xenvif_deinit_queue(&vif->queues[queue_index]);
> >>
> >> -  spin_lock(&vif->lock);
> >> -  queues = vif->queues;
> >>vif->num_queues = 0;
> >> -  vif->queues = NULL;
> >> -  spin_unlock(&vif->lock);
> >> +  synchronize_net();
> >
> > So, num_queues is your RCU protected value, rather than the queues
> pointer, in which case I think you probably need to change code such as
> >
> >
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/ne
> t/xen-netback/netback.c?id=refs/tags/v4.10#n216
> >
> > to be gated on num_queues.
> >
> > Also shouldn't xenvif_up(), xenvif_down() and xenvif_get_ethtool_stats()
> not be using rcu_read_lock() and rcu_dereference() of num_queues as well?
> >
> >   Paul
> >
> >>
> >> -  vfree(queues);
> >> +  for (queue_index = 0; queue_index < num_queues;
> >> ++queue_index)
> >> + 

Re: [Xen-devel] [PATCH] xen-netback: fix race condition on XenBus disconnect

[Paul Durrant]

> -Original Message-
> From: Igor Druzhinin
> Sent: 03 March 2017 13:54
> To: Paul Durrant ; net...@vger.kernel.org; xen-
> de...@lists.xenproject.org
> Cc: jgr...@suse.com; Wei Liu 
> Subject: Re: [PATCH] xen-netback: fix race condition on XenBus disconnect
> 
> On 03/03/17 09:18, Paul Durrant wrote:
> >> -Original Message-
> >> From: Igor Druzhinin [mailto:igor.druzhi...@citrix.com]
> >> Sent: 02 March 2017 22:57
> >> To: net...@vger.kernel.org; xen-de...@lists.xenproject.org
> >> Cc: Paul Durrant ; jgr...@suse.com; Wei Liu
> >> ; Igor Druzhinin 
> >> Subject: [PATCH] xen-netback: fix race condition on XenBus disconnect
> >>
> >> In some cases during XenBus disconnect event handling and subsequent
> >> queue resource release there may be some TX handlers active on
> >> other processors. Use RCU in order to synchronize with them.
> >>
> >> Signed-off-by: Igor Druzhinin 
> >> ---
> >>  drivers/net/xen-netback/interface.c | 13 -
> >>  drivers/net/xen-netback/xenbus.c| 17 +++--
> >>  2 files changed, 15 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-
> >> netback/interface.c
> >> index a2d32676..32e2cc6 100644
> >> --- a/drivers/net/xen-netback/interface.c
> >> +++ b/drivers/net/xen-netback/interface.c
> >> @@ -164,7 +164,7 @@ static int xenvif_start_xmit(struct sk_buff *skb,
> struct
> >> net_device *dev)
> >>  {
> >>struct xenvif *vif = netdev_priv(dev);
> >>struct xenvif_queue *queue = NULL;
> >> -  unsigned int num_queues = vif->num_queues;
> >
> > Do you not need an rcu_read_lock() around this and use of the
> num_queues value (as you have below)?
> >
> >> +  unsigned int num_queues = rcu_dereference(vif)->num_queues;
> >>u16 index;
> >>struct xenvif_rx_cb *cb;
> >>
> >> @@ -221,18 +221,21 @@ static struct net_device_stats
> >> *xenvif_get_stats(struct net_device *dev)
> >>  {
> >>struct xenvif *vif = netdev_priv(dev);
> >>struct xenvif_queue *queue = NULL;
> >> +  unsigned int num_queues;
> >>u64 rx_bytes = 0;
> >>u64 rx_packets = 0;
> >>u64 tx_bytes = 0;
> >>u64 tx_packets = 0;
> >>unsigned int index;
> >>
> >> -  spin_lock(&vif->lock);
> >> -  if (vif->queues == NULL)
> >> +  rcu_read_lock();
> >> +
> >> +  num_queues = rcu_dereference(vif)->num_queues;
> >> +  if (num_queues < 1)
> >>goto out;
> >>
> >>/* Aggregate tx and rx stats from each queue */
> >> -  for (index = 0; index < vif->num_queues; ++index) {
> >> +  for (index = 0; index < num_queues; ++index) {
> >>queue = &vif->queues[index];
> >>rx_bytes += queue->stats.rx_bytes;
> >>rx_packets += queue->stats.rx_packets;
> >> @@ -241,7 +244,7 @@ static struct net_device_stats
> >> *xenvif_get_stats(struct net_device *dev)
> >>}
> >>
> >>  out:
> >> -  spin_unlock(&vif->lock);
> >> +  rcu_read_unlock();
> >>
> >>vif->dev->stats.rx_bytes = rx_bytes;
> >>vif->dev->stats.rx_packets = rx_packets;
> >> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-
> >> netback/xenbus.c
> >> index d2d7cd9..76efb01 100644
> >> --- a/drivers/net/xen-netback/xenbus.c
> >> +++ b/drivers/net/xen-netback/xenbus.c
> >> @@ -495,26 +495,23 @@ static void backend_disconnect(struct
> >> backend_info *be)
> >>struct xenvif *vif = be->vif;
> >>
> >>if (vif) {
> >> +  unsigned int num_queues = vif->num_queues;
> >>unsigned int queue_index;
> >> -  struct xenvif_queue *queues;
> >>
> >>xen_unregister_watchers(vif);
> >>  #ifdef CONFIG_DEBUG_FS
> >>xenvif_debugfs_delif(vif);
> >>  #endif /* CONFIG_DEBUG_FS */
> >>xenvif_disconnect_data(vif);
> >> -  for (queue_index = 0;
> >> -   queue_index < vif->num_queues;
> >> -   ++queue_index)
> >> -  xenvif_deinit_queue(&vif->queues[queue_index]);
> >>
> >> -  spin_lock(&vif->lock);
> >> -  queues = vif->queues;
> >>vif->num_queues = 0;
> >> -  vif->queues = NULL;
> >> -  spin_unlock(&vif->lock);
> >> +  synchronize_net();
> >
> > So, num_queues is your RCU protected value, rather than the queues
> pointer, in which case I think you probably need to change code such as
> >
> >
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/ne
> t/xen-netback/netback.c?id=refs/tags/v4.10#n216
> >
> > to be gated on num_queues.
> >
> > Also shouldn't xenvif_up(), xenvif_down() and xenvif_get_ethtool_stats()
> not be using rcu_read_lock() and rcu_dereference() of num_queues as well?
> >
> >   Paul
> >
> 
> xenvif_up() and xenvif_down() are called under rtnl_lock.
> xenvif_get_ethtool_stats() is a good candidate. I'm not sure about
> xenvif_fatal_tx_err. Is it actually called from already protected regions?
> 

IIRC it's called out of the NAPI worker so I'd hope turning carrier off would 
be sufficient. I still thing that testing for !n

[Xen-devel] [qemu-mainline test] 106395: tolerable FAIL - PUSHED

[osstest service owner]

flight 106395 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/106395/

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 106356
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 106356
 test-armhf-armhf-libvirt-raw 12 saverestore-support-checkfail  like 106356
 test-armhf-armhf-libvirt 13 saverestore-support-checkfail  like 106356
 test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail  like 106356
 test-amd64-amd64-xl-rtds  9 debian-install   fail  like 106356

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-rtds  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 build-arm64   5 xen-buildfail   never pass
 build-arm64-xsm   5 xen-buildfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 build-arm64-pvops 5 kernel-build fail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail 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-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl  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-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-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-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-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass

version targeted for testing:
 qemuuecb24d334af1a98ef0329f4b3b0e14ae8cb8770d
baseline version:
 qemuu1e0addb682c3c552fd97480037d4f8ff18e2b87e

Last test of basis   106356  2017-03-02 07:03:28 Z1 days
Failing since106377  2017-03-02 18:18:07 Z0 days2 attempts
Testing same since   106395  2017-03-03 04:58:49 Z0 days1 attempts


People who touched revisions under test:
  Alexey Kardashevskiy 
  Andrea Bolognani 
  Ashijeet Acharya 
  Christian Borntraeger 
  Cédric Le Goater 
  Daniel Henrique Barboza 
  David Gibson 
  Dr. David Alan Gilbert 
  Eduardo Habkost 
  Fam Zheng 
  Gerd Hoffmann 
  Greg Kurz 
  Greg Kurz 
  Halil Pasic 
  Igor Mammedov 
  Jiri Denemark 
  Kevin Wolf 
  Laurent Vivier 
  Marc-André Lureau 
  Mark Cave-Ayland 
  Markus Armbruster 
  Max Reitz 
  Mike Nawrocki 
  Nikunj A Dadhania 
  Pavel Dovgalyuk 
  Peter Lieven 
  Peter Maydell 
  Pranith Kumar 
  

[Xen-devel] [PATCH] x86/tboot: remove dead declarations

[Jan Beulich]

These aren't needed anymore as of c9a4a1c419 ("x86/layout: Correct
Xen's idea of its own memory layout").

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/tboot.c
+++ b/xen/arch/x86/tboot.c
@@ -48,8 +48,6 @@ static uint64_t __initdata sinit_base, _
 #define TXTCR_HEAP_BASE 0x0300
 #define TXTCR_HEAP_SIZE 0x0308
 
-extern char __init_begin[], __bss_start[], __bss_end[];
-
 #define SHA1_SIZE  20
 typedef uint8_t   sha1_hash_t[SHA1_SIZE];
 



x86/tboot: remove dead declarations

These aren't needed anymore as of c9a4a1c419 ("x86/layout: Correct
Xen's idea of its own memory layout").

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/tboot.c
+++ b/xen/arch/x86/tboot.c
@@ -48,8 +48,6 @@ static uint64_t __initdata sinit_base, _
 #define TXTCR_HEAP_BASE 0x0300
 #define TXTCR_HEAP_SIZE 0x0308
 
-extern char __init_begin[], __bss_start[], __bss_end[];
-
 #define SHA1_SIZE  20
 typedef uint8_t   sha1_hash_t[SHA1_SIZE];
 
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2] x86/SVM: correct boot time cpu_data[] handling

[Jan Beulich]

start_svm() already runs after cpu_data[] was set up, so it shouldn't
modify it anymore (at least not directly). Constify the involved
pointers.

Furthermore LMSLE feature detection was broken by 566ddbe833 ("x86:
Fail CPU bringup cleanly if it cannot initialise HVM"), as Andrew
Cooper has pointed out: c couldn't possibly equal &boot_cpu_data
anymore.

Signed-off-by: Jan Beulich 
---
v2: Suppress setting cpu_has_lmsl for now.

--- a/xen/arch/x86/hvm/svm/asid.c
+++ b/xen/arch/x86/hvm/svm/asid.c
@@ -22,7 +22,7 @@
 #include 
 #include 
 
-void svm_asid_init(struct cpuinfo_x86 *c)
+void svm_asid_init(const struct cpuinfo_x86 *c)
 {
 int nasids = 0;
 
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1330,7 +1330,7 @@ static int svm_cpu_up_prepare(unsigned i
 return 0;
 }
 
-static void svm_init_erratum_383(struct cpuinfo_x86 *c)
+static void svm_init_erratum_383(const struct cpuinfo_x86 *c)
 {
 uint64_t msr_content;
 
@@ -1365,11 +1365,12 @@ static int svm_handle_osvw(struct vcpu *
 return 0;
 }
 
-static int svm_cpu_up(void)
+static int _svm_cpu_up(bool bsp)
 {
 uint64_t msr_content;
-int rc, cpu = smp_processor_id();
-struct cpuinfo_x86 *c = &cpu_data[cpu];
+int rc;
+unsigned int cpu = smp_processor_id();
+const struct cpuinfo_x86 *c = &cpu_data[cpu];
  
 /* Check whether SVM feature is disabled in BIOS */
 rdmsrl(MSR_K8_VM_CR, msr_content);
@@ -1402,7 +1403,7 @@ static int svm_cpu_up(void)
 rdmsrl(MSR_EFER, msr_content);
 if ( msr_content & EFER_LMSLE )
 {
-if ( c == &boot_cpu_data )
+if ( 0 && /* FIXME: Migration! */ bsp )
 cpu_has_lmsl = 1;
 wrmsrl(MSR_EFER, msr_content ^ EFER_LMSLE);
 }
@@ -1419,13 +1420,18 @@ static int svm_cpu_up(void)
 return 0;
 }
 
+static int svm_cpu_up(void)
+{
+return _svm_cpu_up(false);
+}
+
 const struct hvm_function_table * __init start_svm(void)
 {
 bool_t printed = 0;
 
 svm_host_osvw_reset();
 
-if ( svm_cpu_up() )
+if ( _svm_cpu_up(true) )
 {
 printk("SVM: failed to initialise.\n");
 return NULL;
--- a/xen/include/asm-x86/hvm/svm/asid.h
+++ b/xen/include/asm-x86/hvm/svm/asid.h
@@ -22,7 +22,7 @@
 #include 
 #include 
 
-void svm_asid_init(struct cpuinfo_x86 *c);
+void svm_asid_init(const struct cpuinfo_x86 *c);
 
 static inline void svm_asid_g_invlpg(struct vcpu *v, unsigned long g_vaddr)
 {



x86/SVM: correct boot time cpu_data[] handling

start_svm() already runs after cpu_data[] was set up, so it shouldn't
modify it anymore (at least not directly). Constify the involved
pointers.

Furthermore LMSLE feature detection was broken by 566ddbe833 ("x86:
Fail CPU bringup cleanly if it cannot initialise HVM"), as Andrew
Cooper has pointed out: c couldn't possibly equal &boot_cpu_data
anymore.

Signed-off-by: Jan Beulich 
---
v2: Suppress setting cpu_has_lmsl for now.

--- a/xen/arch/x86/hvm/svm/asid.c
+++ b/xen/arch/x86/hvm/svm/asid.c
@@ -22,7 +22,7 @@
 #include 
 #include 
 
-void svm_asid_init(struct cpuinfo_x86 *c)
+void svm_asid_init(const struct cpuinfo_x86 *c)
 {
 int nasids = 0;
 
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1330,7 +1330,7 @@ static int svm_cpu_up_prepare(unsigned i
 return 0;
 }
 
-static void svm_init_erratum_383(struct cpuinfo_x86 *c)
+static void svm_init_erratum_383(const struct cpuinfo_x86 *c)
 {
 uint64_t msr_content;
 
@@ -1365,11 +1365,12 @@ static int svm_handle_osvw(struct vcpu *
 return 0;
 }
 
-static int svm_cpu_up(void)
+static int _svm_cpu_up(bool bsp)
 {
 uint64_t msr_content;
-int rc, cpu = smp_processor_id();
-struct cpuinfo_x86 *c = &cpu_data[cpu];
+int rc;
+unsigned int cpu = smp_processor_id();
+const struct cpuinfo_x86 *c = &cpu_data[cpu];
  
 /* Check whether SVM feature is disabled in BIOS */
 rdmsrl(MSR_K8_VM_CR, msr_content);
@@ -1402,7 +1403,7 @@ static int svm_cpu_up(void)
 rdmsrl(MSR_EFER, msr_content);
 if ( msr_content & EFER_LMSLE )
 {
-if ( c == &boot_cpu_data )
+if ( 0 && /* FIXME: Migration! */ bsp )
 cpu_has_lmsl = 1;
 wrmsrl(MSR_EFER, msr_content ^ EFER_LMSLE);
 }
@@ -1419,13 +1420,18 @@ static int svm_cpu_up(void)
 return 0;
 }
 
+static int svm_cpu_up(void)
+{
+return _svm_cpu_up(false);
+}
+
 const struct hvm_function_table * __init start_svm(void)
 {
 bool_t printed = 0;
 
 svm_host_osvw_reset();
 
-if ( svm_cpu_up() )
+if ( _svm_cpu_up(true) )
 {
 printk("SVM: failed to initialise.\n");
 return NULL;
--- a/xen/include/asm-x86/hvm/svm/asid.h
+++ b/xen/include/asm-x86/hvm/svm/asid.h
@@ -22,7 +22,7 @@
 #include 
 #include 
 
-void svm_asid_init(struct cpuinfo_x86 *c);
+void svm_asid_init(const struct cpuinfo_x86 *c);
 
 static inline void svm_asid_g_invlpg(struct vcpu *v, unsigned long g_vaddr)
 {
___
Xen-devel mailing list
Xen-devel@lis

[Xen-devel] [PATCH] AMD-Vi: allocate root table on demand

[Jan Beulich]

This was my originally intended fix for the AMD side of XSA-207:
There's no need to unconditionally allocate the root table, and with
that there's then also no way to leak it when a guest has no devices
assigned.

Signed-off-by: Jan Beulich 

--- a/xen/drivers/passthrough/amd/iommu_map.c
+++ b/xen/drivers/passthrough/amd/iommu_map.c
@@ -636,11 +636,10 @@ int amd_iommu_map_page(struct domain *d,
 {
 bool_t need_flush = 0;
 struct domain_iommu *hd = dom_iommu(d);
+int rc;
 unsigned long pt_mfn[7];
 unsigned int merge_level;
 
-BUG_ON( !hd->arch.root_table );
-
 if ( iommu_use_hap_pt(d) )
 return 0;
 
@@ -648,6 +647,13 @@ int amd_iommu_map_page(struct domain *d,
 
 spin_lock(&hd->arch.mapping_lock);
 
+rc = amd_iommu_alloc_root(hd);
+if ( rc )
+{
+spin_unlock(&hd->arch.mapping_lock);
+return rc;
+}
+
 /* Since HVM domain is initialized with 2 level IO page table,
  * we might need a deeper page table for lager gfn now */
 if ( is_hvm_domain(d) )
@@ -717,8 +723,6 @@ int amd_iommu_unmap_page(struct domain *
 unsigned long pt_mfn[7];
 struct domain_iommu *hd = dom_iommu(d);
 
-BUG_ON( !hd->arch.root_table );
-
 if ( iommu_use_hap_pt(d) )
 return 0;
 
@@ -726,6 +730,12 @@ int amd_iommu_unmap_page(struct domain *
 
 spin_lock(&hd->arch.mapping_lock);
 
+if ( !hd->arch.root_table )
+{
+spin_unlock(&hd->arch.mapping_lock);
+return 0;
+}
+
 /* Since HVM domain is initialized with 2 level IO page table,
  * we might need a deeper page table for lager gfn now */
 if ( is_hvm_domain(d) )
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -222,23 +222,29 @@ int __init amd_iov_detect(void)
 return scan_pci_devices();
 }
 
-static int allocate_domain_resources(struct domain_iommu *hd)
+int amd_iommu_alloc_root(struct domain_iommu *hd)
 {
-/* allocate root table */
-spin_lock(&hd->arch.mapping_lock);
-if ( !hd->arch.root_table )
+if ( unlikely(!hd->arch.root_table) )
 {
 hd->arch.root_table = alloc_amd_iommu_pgtable();
 if ( !hd->arch.root_table )
-{
-spin_unlock(&hd->arch.mapping_lock);
 return -ENOMEM;
-}
 }
-spin_unlock(&hd->arch.mapping_lock);
+
 return 0;
 }
 
+static int __must_check allocate_domain_resources(struct domain_iommu *hd)
+{
+int rc;
+
+spin_lock(&hd->arch.mapping_lock);
+rc = amd_iommu_alloc_root(hd);
+spin_unlock(&hd->arch.mapping_lock);
+
+return rc;
+}
+
 static int get_paging_mode(unsigned long entries)
 {
 int level = 1;
@@ -259,14 +265,6 @@ static int amd_iommu_domain_init(struct
 {
 struct domain_iommu *hd = dom_iommu(d);
 
-/* allocate page directroy */
-if ( allocate_domain_resources(hd) != 0 )
-{
-if ( hd->arch.root_table )
-free_domheap_page(hd->arch.root_table);
-return -ENOMEM;
-}
-
 /* For pv and dom0, stick with get_paging_mode(max_page)
  * For HVM dom0, use 2 level page table at first */
 hd->arch.paging_mode = is_hvm_domain(d) ?
@@ -280,6 +278,9 @@ static void __hwdom_init amd_iommu_hwdom
 unsigned long i; 
 const struct amd_iommu *iommu;
 
+if ( allocate_domain_resources(dom_iommu(d)) )
+BUG();
+
 if ( !iommu_passthrough && !need_iommu(d) )
 {
 int rc = 0;
@@ -363,7 +364,7 @@ static int reassign_device(struct domain
u8 devfn, struct pci_dev *pdev)
 {
 struct amd_iommu *iommu;
-int bdf;
+int bdf, rc;
 struct domain_iommu *t = dom_iommu(target);
 
 bdf = PCI_BDF2(pdev->bus, pdev->devfn);
@@ -385,10 +386,9 @@ static int reassign_device(struct domain
 pdev->domain = target;
 }
 
-/* IO page tables might be destroyed after pci-detach the last device
- * In this case, we have to re-allocate root table for next pci-attach.*/
-if ( t->arch.root_table == NULL )
-allocate_domain_resources(t);
+rc = allocate_domain_resources(t);
+if ( rc )
+return rc;
 
 amd_iommu_setup_domain_device(target, iommu, devfn, pdev);
 AMD_IOMMU_DEBUG("Re-assign %04x:%02x:%02x.%u from dom%d to dom%d\n",
--- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
+++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
@@ -56,6 +56,7 @@ int __must_check amd_iommu_map_page(stru
 unsigned long mfn, unsigned int flags);
 int __must_check amd_iommu_unmap_page(struct domain *d, unsigned long gfn);
 u64 amd_iommu_get_next_table_from_pte(u32 *entry);
+int __must_check amd_iommu_alloc_root(struct domain_iommu *hd);
 int amd_iommu_reserve_domain_unity_map(struct domain *domain,
u64 phys_addr, unsigned long size,
int iw, int ir);


AMD-Vi: allocate root table on demand

This was my originally intended fix for the AMD si

Re: [Xen-devel] [PATCH] x86/tboot: remove dead declarations

[Andrew Cooper]

On 03/03/17 14:27, Jan Beulich wrote:
> These aren't needed anymore as of c9a4a1c419 ("x86/layout: Correct
> Xen's idea of its own memory layout").
>
> Signed-off-by: Jan Beulich 

Reviewed-by: Andrew Cooper 

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

Re: [Xen-devel] [PATCH v2] x86/SVM: correct boot time cpu_data[] handling

[Andrew Cooper]

On 03/03/17 14:28, Jan Beulich wrote:
> start_svm() already runs after cpu_data[] was set up, so it shouldn't
> modify it anymore (at least not directly). Constify the involved
> pointers.
>
> Furthermore LMSLE feature detection was broken by 566ddbe833 ("x86:
> Fail CPU bringup cleanly if it cannot initialise HVM"), as Andrew
> Cooper has pointed out: c couldn't possibly equal &boot_cpu_data
> anymore.
>
> Signed-off-by: Jan Beulich 

Reviewed-by: Andrew Cooper 

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

Re: [Xen-devel] [PATCH RFC 3/5] x86: split PV dom0 builder to domain_build_pv.c

[Jan Beulich]

>>> On 03.03.17 at 14:35,  wrote:
> On Fri, Mar 03, 2017 at 04:15:19AM -0700, Jan Beulich wrote:
>> >>> On 03.03.17 at 12:06,  wrote:
>> > On Fri, Mar 03, 2017 at 09:41:09AM +, Wei Liu wrote:
>> However, with ...
>> 
>> >> diff --git a/xen/arch/x86/domain_build_pv.c 
>> >> b/xen/arch/x86/domain_build_pv.c
>> > 
>> > I would place this in arch/xen/x86/pv/domain_build.c
>> 
>> ... this the case becomes less clear: Personally I'm not a fan of
>> #include "../file.h", so I wouldn't be certain which of the two is
>> the less undesirable place for the header.
>> 
> 
> But in this case I would prefer to move the to xen/include/xen and named
> in dom0_build.h.

Well, it's x86_specific, so if anything include/asm-x86/.

Jan


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

Re: [Xen-devel] [PATCH RFC 0/5] Refactor x86 domain builder

[Jan Beulich]

>>> On 03.03.17 at 14:41,  wrote:
> On Fri, Mar 03, 2017 at 04:06:00AM -0700, Jan Beulich wrote:
>> >>> On 03.03.17 at 11:56,  wrote:
>> > On 03/03/17 09:41, Wei Liu wrote:
>> > Jan: Would you be happy accepting my CONFIG_{PV,HVM} patch in its silent
>> > form for now?  If so, I will respin it.
>> 
>> I think so, yes.
>> 
>> >> This series is built on top of Roger's PVHv1 removal series.
>> >>
>> >> Wei Liu (5):
>> >>   xen: move round_pg{up,down} to pfn.h
>> >>   xen: include xen/types.h in domain.h
>> >>   x86: split PV dom0 builder to domain_build_pv.c
>> >>   x86: split PVH dom0 builder to domain_build_pv.c
>> >>   x86: clean up header files in domain_build.c
>> > 
>> > I'd argue that these should be named differently.  They are specifically
>> > hwdom_build.c, distinct from general domain building operations.
>> 
>> hwdom_build.c is wrong too - this is strictly Dom0, i.e. it ought
>> to be dom0_build.c.
> 
> Yes, I agree.
> 
> We shall have dom0_build.c, dom0_build_pv.c and dom0_build_pvh.c in v2.

You mean

dom0_build.c
hvm/dom0_build.c
pv/dom0_build.c

Jan


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

Re: [Xen-devel] [PATCH v2 1/5] golang/xenlight: Create stub package

[George Dunlap]

On 02/03/17 16:07, Ronald Rojas wrote:
> Create a basic Makefile to build and install libxenlight Golang
> bindings. Also add a stub package which only opens libxl context.
> 
> Include a global xenlight.Ctx variable which can be used as the
> default context by the entire program if desired.
> 
> For now, return simple errors. Proper error handling will be
> added in next patch.
> 
> Signed-off-by: Ronald Rojas 
> Signed-off-by: George Dunlap 

Getting close!  A few more changes.

> ---
> 
> Changes:
> - Changed GPL Lisense to LGPL Lisense
> 
> - Initialized xentoollog_logger for storing error messages
> 
> - Moved manual-enable config option to tools/Rules.mk, use
>   CONFIG_GOLANG in tools/Makefile
> 
> - Added XEN_GOPATH, pointing to tools/golang
> 
> - Modified tools/golang/xenlight makefile to construct necessary $GOPATH
> 
> - Added tools/golang/Makefile, so we don't need special rules in tools
>   to make tools/golang/xenlight; and so we have a single place to remove the
>   $GOPATH build side-effects ($GOPATH/src and $GOPATH/pkg)
> 
> - Build of tools/golang/xenlight sets $GOPATH and does a 'go install'
> 
> - Use tree-provided CFLAGS_libxenlight and LDLIBS_libxenlight, rather
>   than hard-coding our own
> 
> - Made a PKGSOURCES variable to track dependencies of all target files
>   which need to be part of the output package (i.e., what's put in
>   $GOPATH/src).  At the moment this is one file, but it will probably
>   be more once we start using the IDL.

Good summary of changes, thanks.

> 
> CC: xen-devel@lists.xen.org
> CC: george.dun...@citrix.com
> CC: ian.jack...@eu.citrix.com
> CC: wei.l...@citrix.com
> ---
> ---
>  tools/Makefile|  1 +
>  tools/Rules.mk|  6 +++
>  tools/golang/Makefile | 27 +
>  tools/golang/xenlight/Makefile| 49 ++
>  tools/golang/xenlight/xenlight.go | 85 
> +++
>  5 files changed, 168 insertions(+)
>  create mode 100644 tools/golang/Makefile
>  create mode 100644 tools/golang/xenlight/Makefile
>  create mode 100644 tools/golang/xenlight/xenlight.go
> 
> diff --git a/tools/Makefile b/tools/Makefile
> index 77e0723..df1fda1 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -31,6 +31,7 @@ endif
>  
>  SUBDIRS-y += xenpmd
>  SUBDIRS-y += libxl
> +SUBDIRS-$(CONFIG_GOLANG) += golang
>  SUBDIRS-y += helpers
>  SUBDIRS-$(CONFIG_X86) += xenpaging
>  SUBDIRS-$(CONFIG_X86) += debugger/gdbsx
> diff --git a/tools/Rules.mk b/tools/Rules.mk
> index b35999b..24e5220 100644
> --- a/tools/Rules.mk
> +++ b/tools/Rules.mk
> @@ -30,6 +30,12 @@ XENSTORE_XENSTORED ?= y
>  debug ?= y
>  debug_symbols ?= $(debug)
>  
> +# Uncomment to compile with Go
> +CONFIG_GOLANG ?= y

You forgot to comment this out again before submitting it. :-)  (This is
something that can be fixed up on check-in if necessary.)

> +ifeq ($(CONFIG_GOLANG),y)
> +XEN_GOPATH= $(XEN_ROOT)/tools/golang
> +endif
> +
>  ifeq ($(debug_symbols),y)
>  CFLAGS += -g3
>  endif
> diff --git a/tools/golang/Makefile b/tools/golang/Makefile
> new file mode 100644
> index 000..47a9235
> --- /dev/null
> +++ b/tools/golang/Makefile
> @@ -0,0 +1,27 @@
> +XEN_ROOT=$(CURDIR)/../..
> +include $(XEN_ROOT)/tools/Rules.mk
> +
> +# In order to link against a package in Go, the package must live in a
> +# directory tree in the way that Go expects.  To make this possible,
> +# there must be a directory such that we can set GOPATH=${dir}, and
> +# the package will be under $GOPATH/src/${full-package-path}.
> +
> +# So we set XEN_GOPATH to $XEN_ROOT/tools/golang.  The xenlight
> +# "package build" directory ($PWD/xenlight) will create the "package
> +# source" directory in the proper place.  Go programs can use this
> +# package by setting GOPATH=$(XEN_GOPATH).
> +
> +SUBDIRS-y = xenlight
> +
> +.PHONY: build all
> +all build: subdirs-all
> +
> +.PHONY: install
> +install: subdirs-install
> +
> +.PHONY: clean
> +clean: subdirs-clean
> + $(RM) -r src pkg
> +
> +.PHONY: distclean
> +distclean: clean
> diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
> new file mode 100644
> index 000..5d578f3
> --- /dev/null
> +++ b/tools/golang/xenlight/Makefile
> @@ -0,0 +1,49 @@
> +XEN_ROOT=$(CURDIR)/../../..
> +include $(XEN_ROOT)/tools/Rules.mk
> +
> +# Standing boldly against convention, we insist on installing the
> +# package source under $(prefix)/share/gocode
> +GOCODE_DIR ?= $(prefix)/share/gocode/
> +GOXL_PKG_DIR = /src/xenproject.org/xenlight/

So I had a chat with Ian Jackson about the logistics of setting up a
repo such that "go get" would Just Work (TM).  What we agreed (if I
remember right) was to use the hostname "golang.xenproject.org" for
those mirrored repos, to make sure there was never any name collisions
between web pages at xenproject.org

So we should probably make a variable (maybe XEN_GOCODE_URL), set it to
"golang.xenproject.org", and then replace all instances o

Re: [Xen-devel] [PATCH RFC 0/5] Refactor x86 domain builder

[Wei Liu]

On Fri, Mar 03, 2017 at 07:34:18AM -0700, Jan Beulich wrote:
> >>> On 03.03.17 at 14:41,  wrote:
> > On Fri, Mar 03, 2017 at 04:06:00AM -0700, Jan Beulich wrote:
> >> >>> On 03.03.17 at 11:56,  wrote:
> >> > On 03/03/17 09:41, Wei Liu wrote:
> >> > Jan: Would you be happy accepting my CONFIG_{PV,HVM} patch in its silent
> >> > form for now?  If so, I will respin it.
> >> 
> >> I think so, yes.
> >> 
> >> >> This series is built on top of Roger's PVHv1 removal series.
> >> >>
> >> >> Wei Liu (5):
> >> >>   xen: move round_pg{up,down} to pfn.h
> >> >>   xen: include xen/types.h in domain.h
> >> >>   x86: split PV dom0 builder to domain_build_pv.c
> >> >>   x86: split PVH dom0 builder to domain_build_pv.c
> >> >>   x86: clean up header files in domain_build.c
> >> > 
> >> > I'd argue that these should be named differently.  They are specifically
> >> > hwdom_build.c, distinct from general domain building operations.
> >> 
> >> hwdom_build.c is wrong too - this is strictly Dom0, i.e. it ought
> >> to be dom0_build.c.
> > 
> > Yes, I agree.
> > 
> > We shall have dom0_build.c, dom0_build_pv.c and dom0_build_pvh.c in v2.
> 
> You mean
> 
> dom0_build.c
> hvm/dom0_build.c
> pv/dom0_build.c
> 

Yes, indeed.

> Jan
> 

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

Re: [Xen-devel] [PATCH RFC 3/5] x86: split PV dom0 builder to domain_build_pv.c

[Wei Liu]

On Fri, Mar 03, 2017 at 07:33:35AM -0700, Jan Beulich wrote:
> >>> On 03.03.17 at 14:35,  wrote:
> > On Fri, Mar 03, 2017 at 04:15:19AM -0700, Jan Beulich wrote:
> >> >>> On 03.03.17 at 12:06,  wrote:
> >> > On Fri, Mar 03, 2017 at 09:41:09AM +, Wei Liu wrote:
> >> However, with ...
> >> 
> >> >> diff --git a/xen/arch/x86/domain_build_pv.c 
> >> >> b/xen/arch/x86/domain_build_pv.c
> >> > 
> >> > I would place this in arch/xen/x86/pv/domain_build.c
> >> 
> >> ... this the case becomes less clear: Personally I'm not a fan of
> >> #include "../file.h", so I wouldn't be certain which of the two is
> >> the less undesirable place for the header.
> >> 
> > 
> > But in this case I would prefer to move the to xen/include/xen and named
> > in dom0_build.h.
> 
> Well, it's x86_specific, so if anything include/asm-x86/.
> 

OK.

> Jan
> 

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

Re: [Xen-devel] [RFC PATCH v1 16/21] ARM: NUMA: Extract proximity from SRAT table

[Vijay Kilari]

On Fri, Mar 3, 2017 at 7:22 PM, Julien Grall  wrote:
>
>
> On 03/03/17 13:50, Vijay Kilari wrote:
>>
>> On Fri, Mar 3, 2017 at 7:14 PM, Julien Grall  wrote:
>
> This is quite disgusting. We should avoid any #ifdef CONFIG_{X86,ARM}
> in
> common header.
>
> Also, x2apic and gicc are respectively x86-specific and arm-specific.
> So
> I
> think we should move the parsing in a separate arch-depend function to
> avoid
> those ifdery.
>
> By that I mean having a function acpi_table_arch_parse_srat that will
> parse
> x2apci on x86 and gicc on ARM. Jan, what do you think?



 Linux also follows similar approach


 https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/linux/acpi.h?id=refs/tags/v4.10#n265
>>>
>>>
>>>
>>> So? What are you trying to prove?
>>>
>>> The linux version is much readable than yours. Anyway, we should limit
>>> CONFIG_{X86,ARM} ifdefery in common code.
>>>
>>> Currently, I see no point to have those ifdefery when it is possible to
>>> add
>>> an arch-depend function.
>>
>>
>> This is because in xen we have another level of config CONFIG_ACPI_NUMA.
>> I have plans to reuse cpu and memory part next revision.
>
>
> This does not explain why you want to do like Linux.

Basically want to reuse xen/drivers/acpi/numa.c which is common for
both x86 and ARM.
If not, then we have move some arch specific as you mentioned.

I have another idea where in, if all the NUMA ACPI code is programmed under
CONFIG_NUMA and only initialization is kept under CONFIG_ACPI_NUMA
similar to x86
then we don't need to pollute this header much and limit the changes.

I will try to implement this and see how simple it can go and let you know. OK?

>
> --
> Julien Grall

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

Re: [Xen-devel] [PATCH v2] x86/SVM: correct boot time cpu_data[] handling

[Boris Ostrovsky]

On 03/03/2017 09:28 AM, Jan Beulich wrote:
> start_svm() already runs after cpu_data[] was set up, so it shouldn't
> modify it anymore (at least not directly). Constify the involved
> pointers.
>
> Furthermore LMSLE feature detection was broken by 566ddbe833 ("x86:
> Fail CPU bringup cleanly if it cannot initialise HVM"), as Andrew
> Cooper has pointed out: c couldn't possibly equal &boot_cpu_data
> anymore.
>
> Signed-off-by: Jan Beulich 

Reviewed-by: Boris Ostrovsky 

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

Re: [Xen-devel] [PATCH v2 2/5] golang/xenlight: Add error constants and standard handling

[George Dunlap]

On 02/03/17 16:07, Ronald Rojas wrote:
> Create error type Errorxl for throwing proper xenlight
> errors.
> 
> Update Ctx functions to throw Errorxl errors.
> 
> Signed-off-by: Ronald Rojas 

There are a couple of `go fmt` changes which should be in the previous
patch (including the stray semicolon and a space for a c-style comment).
 With that fixed:

Reviewed-by: George Dunlap 

> ---
> Changes since last patch:
> 
> - Whitespace fixes
> 
> CC: xen-devel@lists.xen.org
> CC: george.dun...@citrix.com
> CC: ian.jack...@eu.citrix.com
> CC: wei.l...@citrix.com
> ---
> ---
>  tools/golang/xenlight/xenlight.go | 81 
> +--
>  1 file changed, 77 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/golang/xenlight/xenlight.go 
> b/tools/golang/xenlight/xenlight.go
> index 0a0cea2..cbd3527 100644
> --- a/tools/golang/xenlight/xenlight.go
> +++ b/tools/golang/xenlight/xenlight.go
> @@ -29,17 +29,79 @@ import "C"
>   *
>   * To get back to static linking:
>   * #cgo LDFLAGS: -lxenlight -lyajl_s -lxengnttab -lxenstore -lxenguest 
> -lxentoollog -lxenevtchn -lxenctrl -lblktapctl -lxenforeignmemory -lxencall 
> -lz -luuid -lutil
> -*/
> + */
>  
>  import (
>   "fmt"
>   "unsafe"
>  )
>  
> +/*
> + * Errors
> + */
> +
> +type Error int
> +
> +const (
> + ErrorNonspecific  = Error(-C.ERROR_NONSPECIFIC)
> + ErrorVersion  = Error(-C.ERROR_VERSION)
> + ErrorFail = Error(-C.ERROR_FAIL)
> + ErrorNi   = Error(-C.ERROR_NI)
> + ErrorNomem= Error(-C.ERROR_NOMEM)
> + ErrorInval= Error(-C.ERROR_INVAL)
> + ErrorBadfail  = Error(-C.ERROR_BADFAIL)
> + ErrorGuestTimedout= Error(-C.ERROR_GUEST_TIMEDOUT)
> + ErrorTimedout = Error(-C.ERROR_TIMEDOUT)
> + ErrorNoparavirt   = Error(-C.ERROR_NOPARAVIRT)
> + ErrorNotReady = Error(-C.ERROR_NOT_READY)
> + ErrorOseventRegFail   = Error(-C.ERROR_OSEVENT_REG_FAIL)
> + ErrorBufferfull   = Error(-C.ERROR_BUFFERFULL)
> + ErrorUnknownChild = Error(-C.ERROR_UNKNOWN_CHILD)
> + ErrorLockFail = Error(-C.ERROR_LOCK_FAIL)
> + ErrorJsonConfigEmpty  = Error(-C.ERROR_JSON_CONFIG_EMPTY)
> + ErrorDeviceExists = Error(-C.ERROR_DEVICE_EXISTS)
> + ErrorCheckpointDevopsDoesNotMatch = 
> Error(-C.ERROR_CHECKPOINT_DEVOPS_DOES_NOT_MATCH)
> + ErrorCheckpointDeviceNotSupported = 
> Error(-C.ERROR_CHECKPOINT_DEVICE_NOT_SUPPORTED)
> + ErrorVnumaConfigInvalid   = Error(-C.ERROR_VNUMA_CONFIG_INVALID)
> + ErrorDomainNotfound   = Error(-C.ERROR_DOMAIN_NOTFOUND)
> + ErrorAborted  = Error(-C.ERROR_ABORTED)
> + ErrorNotfound = Error(-C.ERROR_NOTFOUND)
> + ErrorDomainDestroyed  = Error(-C.ERROR_DOMAIN_DESTROYED)
> + ErrorFeatureRemoved   = Error(-C.ERROR_FEATURE_REMOVED)
> +)
> +
> +var errors = [...]string{
> + ErrorNonspecific:  "Non-specific error",
> + ErrorVersion:  "Wrong version",
> + ErrorFail: "Failed",
> + ErrorNi:   "Not Implemented",
> + ErrorNomem:"No memory",
> + ErrorInval:"Invalid argument",
> + ErrorBadfail:  "Bad Fail",
> + ErrorGuestTimedout:"Guest timed out",
> + ErrorTimedout: "Timed out",
> + ErrorNoparavirt:   "No Paravirtualization",
> + ErrorNotReady: "Not ready",
> + ErrorOseventRegFail:   "OS event registration failed",
> + ErrorBufferfull:   "Buffer full",
> + ErrorUnknownChild: "Unknown child",
> + ErrorLockFail: "Lock failed",
> + ErrorJsonConfigEmpty:  "JSON config empty",
> + ErrorDeviceExists: "Device exists",
> + ErrorCheckpointDevopsDoesNotMatch: "Checkpoint devops does not match",
> + ErrorCheckpointDeviceNotSupported: "Checkpoint device not supported",
> + ErrorVnumaConfigInvalid:   "VNUMA config invalid",
> + ErrorDomainNotfound:   "Domain not found",
> + ErrorAborted:  "Aborted",
> + ErrorNotfound: "Not found",
> + ErrorDomainDestroyed:  "Domain destroyed",
> + ErrorFeatureRemoved:   "Feature removed",
> +}
>  
>  /*
>   * Types: Builtins
>   */
> +
>  type Context struct {
>   ctx *C.libxl_ctx
>  }
> @@ -49,6 +111,17 @@ type Context struct {
>   */
>  var Ctx Context
>  
> +func (e Error) Error() string {
> + if 0 < int(e) && int(e) < len(errors) {
> +

[Xen-devel] [PATCH v5 00/17] x86emul: MMX/SSEn support

[Jan Beulich]

This includes support for AVX counterparts of them as well as a few
later SSE additions (basically covering the entire 0f-prefixed opcode
space, but not the 0f38 and 0f3a ones, nor 3dnow).

 1: support most memory accessing MMX/SSE{,2,3} insns
 2: support MMX/SSE{,2,3} moves
 3: support MMX/SSE/SSE2 converts
 4: support {,V}{,U}COMIS{S,D}
 5: support MMX/SSE{,2,4a} insns with only register operands
 6: support {,V}{LD,ST}MXCSR
 7: support {,V}MOVNTDQA
 8: test coverage for SSE/SSE2 insns
 9: honor MMXEXT feature flag
10: add tables for 0f38 and 0f3a extension space
11: support SSSE3 insns
12: support SSE4.1 insns
13: support SSE4.2 insns
14: test coverage for SSE3/SSSE3/SSE4* insns

Partly RFC from here on, as there's testing code still mostly missing,
albeit I'm unsure whether it makes sense to cover each and every
individual instruction.

15: support PCLMULQDQ
16: support AESNI insns
17: support SHA insns

Signed-off-by: Jan Beulich 
---
v5: Address review feedback. See individual patches.



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

Re: [Xen-devel] [PATCH RFC 3/5] x86: split PV dom0 builder to domain_build_pv.c

[Andrew Cooper]

On 03/03/17 14:45, Wei Liu wrote:
> On Fri, Mar 03, 2017 at 07:33:35AM -0700, Jan Beulich wrote:
> On 03.03.17 at 14:35,  wrote:
>>> On Fri, Mar 03, 2017 at 04:15:19AM -0700, Jan Beulich wrote:
>>> On 03.03.17 at 12:06,  wrote:
> On Fri, Mar 03, 2017 at 09:41:09AM +, Wei Liu wrote:
 However, with ...

>> diff --git a/xen/arch/x86/domain_build_pv.c 
>> b/xen/arch/x86/domain_build_pv.c
> I would place this in arch/xen/x86/pv/domain_build.c
 ... this the case becomes less clear: Personally I'm not a fan of
 #include "../file.h", so I wouldn't be certain which of the two is
 the less undesirable place for the header.

>>> But in this case I would prefer to move the to xen/include/xen and named
>>> in dom0_build.h.
>> Well, it's x86_specific, so if anything include/asm-x86/.
>>

On a related tangent, I'd like to clean up our include hierachy, in the
same way Linux has.

I.e.

move include/asm-x86/ to arch/x86/include/asm

and include/asm-arm/ to arch/arm/include/asm

which allows for the removal of all symlink games, which get in the way
of tools like cscope/tags.

Are people amenable to this?  (I seem to recall asking before, and it
not being objected to).

~Andrew

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