Re: [Xen-devel] [RFC PATCH 00/13] Persistent grant maps for xen net drivers

[Yuzhou (C)]

Hi,

About rx zerocopy, I have a question:

If some application make a socket, then listen and accept, the client 
sends packets to it, but it doesn't recv from this socket right now, all 
persistent grant page would be in used.
So other application cannot receive any packets.  Is my guess right or wrong?

YuZhou

-Original Message-
From: xen-devel-boun...@lists.xen.org [mailto:xen-devel-boun...@lists.xen.org] 
On Behalf Of Joao Martins
Sent: Friday, May 22, 2015 6:27 PM
To: Wei Liu
Cc: ian.campb...@citrix.com; net...@vger.kernel.org; david.vra...@citrix.com; 
xen-de...@lists.xenproject.org; boris.ostrov...@oracle.com
Subject: Re: [Xen-devel] [RFC PATCH 00/13] Persistent grant maps for xen net 
drivers


On 19 May 2015, at 17:39, Wei Liu  wrote:

> On Tue, May 12, 2015 at 07:18:24PM +0200, Joao Martins wrote:
> 
>> There have been recently[3] some discussions and issues raised on 
>> persistent grants for the block layer, though the numbers above show 
>> some significant improvements specially on more network intensive 
>> workloads and provide a margin for comparison against future 
>> map/unmap improvements.
>> 
>> Any comments or suggestions are welcome, Thanks!
> 
> Thanks, the numbers certainly look interesting.
> 
> I'm just a bit concerned about the complexity of netback. I've 
> commented on individual patches, we can discuss the issues there.

Thanks a lot for the review! It does add more complexity, mainly for the TX 
path, but I also would like to mention that a portion of this changeset is also 
the persistent grants ops that could potentially live outside.

Joao

>> [1] http://article.gmane.org/gmane.linux.network/249383
>> [2] http://bit.ly/1IhJfXD
>> [3] 
>> http://lists.xen.org/archives/html/xen-devel/2015-02/msg02292.html
>> 
>> Joao Martins (13):
>>  xen-netback: add persistent grant tree ops
>>  xen-netback: xenbus feature persistent support
>>  xen-netback: implement TX persistent grants
>>  xen-netback: implement RX persistent grants
>>  xen-netback: refactor xenvif_rx_action
>>  xen-netback: copy buffer on xenvif_start_xmit()
>>  xen-netback: add persistent tree counters to debugfs
>>  xen-netback: clone skb if skb->xmit_more is set
>>  xen-netfront: move grant_{ref,page} to struct grant
>>  xen-netfront: refactor claim/release grant
>>  xen-netfront: feature-persistent xenbus support
>>  xen-netfront: implement TX persistent grants
>>  xen-netfront: implement RX persistent grants
>> 
>> drivers/net/xen-netback/common.h|  79 
>> drivers/net/xen-netback/interface.c |  78 +++-
>> drivers/net/xen-netback/netback.c   | 873 
>> ++--
>> drivers/net/xen-netback/xenbus.c|  24 +
>> drivers/net/xen-netfront.c  | 362 ---
>> 5 files changed, 1216 insertions(+), 200 deletions(-)
>> 
>> --
>> 2.1.3



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

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

Re: [Xen-devel] Xen BUG at page_alloc.c:1738 (Xen 4.5)

[Jason Fritcher]

On Wed, 20 May 2015, Major Hayden wrote:

> On 05/20/2015 05:41 AM, Jan Beulich wrote:
> > Considering that no-one else is seeing this - is this perhaps connected
> > to you building Xen with pre-release gcc 5.0.1? This is also because in
> > order for the above to indeed occur, mmio_ro_do_page_fault()'s
> > put_page() would need to drop the last reference of a page, yet
> > page_get_owner_and_reference() doesn't obtain a reference when
> > a page is unallocated (and hence unowned), i.e. normally a page
> > would have a refcount of at least 2 here. Hence this would be
> > possible only due to a race, but the exact same race to be observed
> > on different hardware _and_ under an emulator is extremely unlikely.

You could try with the xen.gz file from 
https://copr-be.cloud.fedoraproject.org/results/myoung/xentest/fedora-21-x86_64/xen-4.5.1-0.rc1.fc21/xen-hypervisor-4.5.1-0.rc1.fc21.x86_64.rpm
 

It is roughly the same version of xen but built against Fedora 21 and gcc 
4.9.2. If that works then it probably is gcc 5.
Greetings,

I have run into pretty much the same issue as the original poster.

I am running a recently updated Arch Linux system, with GCC 5.1.0, using UEFI 
and gummiboot to boot. I currently have a build of Xen 4.4.1, built with GCC 
4.9.2 from before my last update, that is functioning correctly on this 
machine. But the builds of Xen 4.5.0, using GCC 5 and mingw64-binutils for the 
EFI binary, are all failing when Xen starts the Linux kernel, with the same 
error mentioned in the subject. Below is the boot log I captured via the serial 
port.

http://pastebin.com/bBC78306

Wondering if my specific toolchain was the issue, I downloaded the Fedora 22 
version of xen-hypervisor and installed its EFI Xen binary over my compiled 
binary and received an identical error message, with slightly different 
addresses in the panic dump. The Fedora version was compiled with GCC 5.0.1. 
Below is the boot log I captured from that binary.

http://pastebin.com/jvg1JazC 

After finding this thread, and specifically, the quoted message above, I 
downloaded that xen-hypervisor package and installed its EFI Xen binary. That 
binary boots successfully, as seen by the captured boot log below.

http://pastebin.com/DKxwaU2U

So, while I’m not familiar enough with Xen to begin to have an idea of what 
could possibly be wrong with Xen or GCC 5 to be causing this bug, I’d like to 
do what I can to track down the issue so I can get a working build of Xen 4.5. 
:)

Thanks!

—
Jason Fritcher



smime.p7s
Description: S/MIME cryptographic signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v6 1/4] pci: add pci_iomap_wc() variants

[Tomi Valkeinen]

On 29/05/15 03:36, Luis R. Rodriguez wrote:
> On Wed, May 27, 2015 at 1:04 PM, Luis R. Rodriguez  wrote:
>> On Tue, May 26, 2015 at 12:40:08PM -0500, Bjorn Helgaas wrote:
>>> On Fri, May 22, 2015 at 02:23:41AM +0200, Luis R. Rodriguez wrote:
 On Thu, May 21, 2015 at 05:33:21PM -0500, Bjorn Helgaas wrote:
>
> I tentatively put this (and the rest of the series) on a pci/resource
> branch.  I'm hoping you'll propose some clarification about
> EXPORT_SYMBOL_GPL().

 EXPORT_SYMBOL_GPL() also serves to ensure only GPL modules can
 only run that code. So for instance although we have "Dual BSD/GPL"
 tags for modules pure "BSD" tags do not exist for module tags and
 cannot run EXPORT_SYMBOL_GPL() code [0]. Also there is some folks
 who do believe tha at run time all kernel modules are GPL [1] [2].
 And to be precise even though the FSF may claim a list of licenses
 are GPL-compatible we cannot rely on this list alone for our own
 goals and if folks want to use our EXPORT_SYMBOL_GPL()s they must
 discuss this on lkml [2].
>>>
>>> By "propose some clarification," I meant that I hoped you would propose a
>>> patch to Documentation/ that would give maintainers some guidance.
>>
>> I *really really* would hate to do so but only because you insist, I'll look
>> into this...
> 
> OK done. Please let me know if there is anything else I can do to
> help. Also as per review with Tomi, the framebuffer maintainer, he
> would prefer for only the required symbols to go through your tree.
> We'd then wait for the next merge window for them to perculate to
> Linus' tree and once there I'd send him a pull request for the
> framebuffer device driver changes alone. So this does mean we'll have
> no users of the symbols for a full release, but again, this is as per
> Tomi's preference. This strategy is also the preference then for the
> pci_iomap_wc() series as well. With that in mind, perhaps the lib
> patch can go in as we'd have no users but we do have a few future
> possible expected users.

I don't have any issue with fbdev changes going through other trees, but
I'd rather do that only if there are good reasons to go that way.

These changes to fbdev drivers look like cleanups, so they are not
critical, right? Does delaying the fbdev changes until the dependencies
are in prevent some other development?

 Tomi



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

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

[osstest service user]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemuu-win7-amd64 15 guest-localmigrate/x10 fail REGR. vs. 
56941

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-sedf-pin  3 host-install(3)   broken pass in 57410
 test-armhf-armhf-libvirt  3 host-install(3)   broken pass in 57410
 test-amd64-amd64-xl-qemuu-win7-amd64 14 guest-localmigrate.2 fail in 57410 
pass in 57454
 test-amd64-i386-xl-qemuu-win7-amd64 12 guest-localmigrate fail in 57410 pass 
in 57454
 test-armhf-armhf-xl-cubietruck 14 guest-start.2 fail pass in 57410

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-libvirt 11 guest-start   fail in 57410 like 56941
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 56898
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 56941

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-sedf-pin 12 migrate-support-check fail in 57410 never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-sedf 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass

version targeted for testing:
 xen  9d5b2b01024d18b2135c1b0deebb8bfbf5dced5f
baseline version:
 xen  ddfe333aef87e6c5f52b84c8beb3277be4663313


People who touched revisions under test:
  Andrew Cooper 
  David Scott 
  Ian Campbell 
  Ian Jackson 
  Jim Fehlig 
  Ross Lagerwall 
  Wei Liu 


jobs:
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 build-amd64-rumpuserxen  pass
 build-i386-rumpuserxen   pass
 test-amd64-amd64-xl  pass
 test-armhf-armhf-xl  pass
 test-amd64-i386-xl   pass
 test-amd64-amd64-xl-pvh-amd  fail
 test-amd64-i386-qemut-rhel6hvm-amd   pass
 test-amd64-i386-qemuu-rhel6hvm-amd   pass
 test-amd64-amd64-xl-qemut-debianhvm-amd64pass
 test-amd64-i386-xl-qemut-debianhvm-amd64 pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-i386-xl-qemuu-debianhvm-amd64 pass
 test-amd64-i386-freebsd10-amd64  pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  pass
 test-amd64-amd64-rumpuserxen-amd64   pass
 test-amd64-amd64-xl-qemut-win7-amd64 fail
 test-amd64-i386-xl-qemut-win7-amd64  fail
 test-amd64-amd64-xl-qemuu-win7-amd64 fail
 test-amd64-i386-xl-qemuu-win7-amd64  fail
 test-armhf-armhf-xl-arndale  pass
 test-amd64-amd64-xl-credit2  pass
 test-armhf-armhf-xl-credit2  pass
 test-armhf-armhf-xl-cubietruck   fail
 test-amd64-i386-freebsd10-i386   pass
 test-amd64-i386-rumpuserxen-i386 

Re: [Xen-devel] [PATCH] Documentation: extend use case for EXPORT_SYMBOL_GPL()

[Christoph Hellwig]

On Fri, May 29, 2015 at 01:10:44AM +0200, Luis R. Rodriguez wrote:
> Great, thanks. This seems to be in alignment with those who have all along 
> said
> they've used EXPORT_SYMBOL() to mean what EXPORT_SYMBOL_GPL() users now use it
> for. Nevertheless -- maintainers should know that some stubborn developers use
> EXPORT_SYMBOL_GPL() for its technical merit should violators abuse those
> symbols.

FYI, I think the naming here is really unfortunate.  If if was named
EXPORT_SYMBOL_INTERNAL as just a kernel export for specific uses we'd
be much better off in being able to explain what it actually does.

Even better would e a system were we have specific export groups, e.g.
symbols would be "core" "mm", "vfs", or "legacy_hack_for_drm" and any
consumer would specificly declare which symbol they pull in.

This would have a couple advantages:

 - anyone adding an export needs to think hard into which category
   it falls, and think again if exporting really makes sense
 - it's reasy to review modules to see if they pull in anything
   unexpected.

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

[Xen-devel] [qemu-mainline test] 57448: regressions - FAIL

[osstest service user]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-freebsd10-amd64 16 guest-localmigrate/x10 fail in 57362 REGR. 
vs. 56831

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-cubietruck 3 host-install(3) broken in 57362 pass in 57448
 test-armhf-armhf-xl-sedf-pin  3 host-install(3)  broken in 57362 pass in 57448
 test-amd64-i386-freebsd10-i386  6 xen-boot fail in 57362 pass in 57448
 test-amd64-i386-freebsd10-amd64 12 guest-saverestorefail pass in 57362

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-libvirt 11 guest-start   fail in 57362 like 56784
 test-amd64-amd64-xl-qemuu-win7-amd64 9 windows-install fail in 57362 like 56784
 test-amd64-i386-libvirt  11 guest-start  fail   like 56831
 test-amd64-amd64-libvirt 11 guest-start  fail   like 56831

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail never 
pass
 test-amd64-i386-libvirt-xsm  11 guest-start  fail   never pass
 test-amd64-i386-xl-xsm   11 guest-start  fail   never pass
 test-amd64-amd64-xl-xsm  11 guest-start  fail   never pass
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail never 
pass
 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-amd64-libvirt-xsm 11 guest-start  fail   never pass
 test-armhf-armhf-xl-xsm  11 guest-start  fail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-sedf 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 11 guest-start  fail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-sedf-pin 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail never pass
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail never pass

version targeted for testing:
 qemuu0915aed5842bd4dbe396b92d4f3b846ae29ad663
baseline version:
 qemuueba05e922e8e7f307bc5d4104a78797e55124e97


People who touched revisions under test:
  Alberto Garcia 
  Bastian Koppelmann 
  Christoph Hellwig 
  Cole Robinson 
  Daniel P. Berrange 
  Denis V. Lunev 
  Eric Blake 
  Fam Zheng 
  Gerd Hoffmann 
  John Snow 
  Ján Tomko 
  Keith Busch 
  Kevin Wolf 
  Mark Cave-Ayland 
  Mark Cave-Ayland 
  Markus Armbruster 
  Paolo Bonzini 
  Peter Maydell 
  Richard Henderson 
  Richard W.M. Jones 
  Roman Kagan 
  Stefan Hajnoczi 


jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl  pass
 test-armhf-armhf-xl  pass
 test-amd64-i386-xl   pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsmfail
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm fail
 test-amd64-amd64-libvirt-xsm fail
 test-armhf-armhf-libvirt-xsm fail
 test-amd64-i386-libvirt-xsm  fail
 test-amd64-amd64-xl-xsm  fail
 test-armhf-armhf-xl-xsm  fail
 test-amd64-i386-xl-xsm   fail
 test-amd64-am

Re: [Xen-devel] [PATCH v2 5/9] x86/intel_pstate: relocate the driver register/unregister function

[Wang, Wei W]

On 26/05/2015 21:17, Jan Beulich wrote
> >>> On 13.05.16 at 09:50,  wrote:
> > +extern int cpufreq_register_driver(struct cpufreq_driver
> > +*driver_data); extern int cpufreq_unregister_driver(struct
> > +cpufreq_driver *driver);
> 
> Oh, btw, please also get rid of "extern" on function declarations, unless in a
> particular header it is consistently being used.

Ok. The cpufreq.h file uses "extern", so I will keep using it.

Best,
Wei

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

Re: [Xen-devel] [PATCH v2 5/9] x86/intel_pstate: relocate the driver register/unregister function

[Wang, Wei W]

On 26/05/2015 21:06, Jan Beulich wrote
> >>> On 13.05.16 at 09:50,  wrote:
> > Register/unregister the CPU hotplug notifier when the driver is
> > registered, and move the driver register/unregister function to the
> > cpufreq.c.
> 
> Without saying why I'm afraid I don't even see much reason to review this in
> any detail.
> 
> > --- a/xen/drivers/cpufreq/cpufreq.c
> > +++ b/xen/drivers/cpufreq/cpufreq.c
> > @@ -630,12 +630,31 @@ static struct notifier_block cpu_nfb = {
> >  .notifier_call = cpu_callback
> >  };
> >
> > -static int __init cpufreq_presmp_init(void)
> > +int cpufreq_register_driver(struct cpufreq_driver *driver_data)
> >  {
> > -void *cpu = (void *)(long)smp_processor_id();
> > -cpu_callback(&cpu_nfb, CPU_ONLINE, cpu);
> 
> Why is this being removed without replacement?

I think they are redundant here. 
If we go and check the hypercall code path (the bottom of set_px_pminfo()),  
the cpufreq_add_cpu() is called there (inside cpufreq_cpu_init()), too. Why do 
we need to initialize this CPU twice?

Best,
Wei


> > +if (!driver_data || !driver_data->init
> > +|| !driver_data->verify || !driver_data->exit
> > +|| (!driver_data->target == !driver_data->setpolicy))
> > +return -EINVAL;
> > +
> > +if (cpufreq_driver)
> > +return -EBUSY;
> > +
> > +cpufreq_driver = driver_data;
> > +
> >  register_cpu_notifier(&cpu_nfb);
> > +
> >  return 0;
> >  }
> > -presmp_initcall(cpufreq_presmp_init);
> >
> > +int cpufreq_unregister_driver(struct cpufreq_driver *driver) {
> > +if (!cpufreq_driver || (driver != cpufreq_driver))
> > +return -EINVAL;
> > +
> > +cpufreq_driver = NULL;
> > +
> > +unregister_cpu_notifier(&cpu_nfb);
> > +
> > +return 0;
> > +}
> 
> This function is dead (and perhaps just like unregister_cpu_notifier() not
> even needed without loadable modules).
> 
> Jan


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

Re: [Xen-devel] [PATCH v8 05/13] x86: expose CBM length and COS number information

[Chao Peng]

On Thu, May 28, 2015 at 02:26:03PM +0100, Jan Beulich wrote:
> 
> > --- a/xen/include/public/sysctl.h
> > +++ b/xen/include/public/sysctl.h
> > @@ -694,6 +694,20 @@ struct xen_sysctl_pcitopoinfo {
> >  typedef struct xen_sysctl_pcitopoinfo xen_sysctl_pcitopoinfo_t;
> >  DEFINE_XEN_GUEST_HANDLE(xen_sysctl_pcitopoinfo_t);
> >  
> > +#define XEN_SYSCTL_PSR_CAT_get_l3_info   0
> > +struct xen_sysctl_psr_cat_op {
> > +uint32_t cmd;   /* IN: XEN_SYSCTL_PSR_CAT_* */
> > +uint32_t target;/* IN: socket to be operated on */
> 
> If this is always the socket number, why would the variable be
> named anything other than "socket". If otoh subsequent patches
> use it differently, I think the comment should be omitted now
> rather than being dropped then (or it should be given its final
> wording from the beginning).

Or 'target to be operated on'?

Chao

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

Re: [Xen-devel] [PATCH v8 03/13] x86: maintain COS to CBM mapping for each socket

[Chao Peng]

On Thu, May 28, 2015 at 02:17:54PM +0100, Jan Beulich wrote:
> >>> On 21.05.15 at 10:41,  wrote:
> > For each socket, a COS to CBM mapping structure is maintained for each
> > COS. The mapping is indexed by COS and the value is the corresponding
> > CBM. Different VMs may use the same CBM, a reference count is used to
> > indicate if the CBM is available.
> > 
> > Signed-off-by: Chao Peng 
> > Reviewed-by: Andrew Cooper 
> > ---
> > Changes in v8:
> > * Move the memory allocation and CAT initialization code to CPU_UP_PREPARE.
> > * Add memory freeing code in CPU_DEAD path.
> 
> Changes like this imo invalidate any tags given for earlier versions.

Sure, I will remove it.

> > +static int cat_cpu_init(unsigned int cpu)
> > +{
> > +int rc;
> > +const struct cpuinfo_x86 *c = cpu_data + cpu;
> > +
> > +if ( !cpu_has(c, X86_FEATURE_CAT) )
> > +return 0;
> > +
> > +if ( test_bit(cpu_to_socket(cpu), cat_socket_enable) )
> > +return 0;
> > +
> > +if ( cpu == smp_processor_id() )
> > +do_cat_cpu_init(&rc);
> > +else
> > +on_selected_cpus(cpumask_of(cpu), do_cat_cpu_init, &rc, 1);
> 
> This now being called in the context of CPU_UP_PREPARE, I can't see
> how this works at all: Neither would the CPU's cpu_data[] instance be
> initialized by that time, nor would you be able to IPI that CPU, nor can I
> see how the if() branch could ever get entered. Was this tested at all?

Ah, yes! So it sounds really a little difficult to move the memory
allocation from CPU_STARTING to CPU_PREPARA for this case.

Chao

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

Re: [Xen-devel] [PATCH v8 02/13] x86: detect and initialize Intel CAT feature

[Chao Peng]

On Thu, May 28, 2015 at 01:54:39PM +0100, Jan Beulich wrote:
> >>> On 21.05.15 at 10:41,  wrote:
> > +
> > +if ( !cpu_has(c, X86_FEATURE_CAT) )
> > +return;
> > +
> > +socket = cpu_to_socket(cpu);
> > +if ( test_bit(socket, cat_socket_enable) )
> > +return;
> > +
> > +cpuid_count(PSR_CPUID_LEVEL_CAT, 0, &eax, &ebx, &ecx, &edx);
> 
> While one would hope that X86_FEATURE_CAT implies the respective
> CPUID leaf being available, I think explicitly checking this should still
> be done just like is the case elsewhere.

Against cpuid_level?

> 
> > +if ( ebx & PSR_RESOURCE_TYPE_L3 )
> > +{
> > +cpuid_count(PSR_CPUID_LEVEL_CAT, 1, &eax, &ebx, &ecx, &edx);
> > +info = cat_socket_info + socket;
> > +info->cbm_len = (eax & 0x1f) + 1;
> > +info->cos_max = min(opt_cos_max, edx & 0x);
> > +
> > +set_bit(socket, cat_socket_enable);
> > +printk(XENLOG_INFO "CAT: enabled on socket %u, cos_max:%u, 
> > cbm_len:%u\n",
> > +   socket, info->cos_max, info->cbm_len);
> > +}
> > +}
> > +
> > +static void cat_cpu_fini(unsigned int cpu)
> > +{
> > +unsigned int socket = cpu_to_socket(cpu);
> > +
> > +if ( !socket_cpumask[socket] || cpumask_empty(socket_cpumask[socket]) )
> > +clear_bit(socket, cat_socket_enable);
> > +}
> 
> This being called from the CPU_DEAD notification, you now depend
> on cpu_smpboot_free) to run ahead of you. Which isn't the case
> afaict, and even if it happened to be that way you shouldn't rely
> on it without explicitly enforcing ordering between the two by
> setting the priority of on of them to a non-default value.

Yes, seems changing the priority of psr_cpu_callback to 1 is enough.

> 
> > +static void __init init_psr_cat(void)
> > +{
> > +if ( opt_cos_max < 1 )
> > +{
> > +printk(XENLOG_INFO "CAT: disabled, cos_max is too small\n");
> > +return;
> > +}
> 
> Is opt_cos_max == 1 really useful for anything?

That means two COSes are available. cos=0 is reserved and cos=1 can
still be used anyway.

Chao

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

Re: [Xen-devel] [PATCH v8 01/13] x86: add socket_cpumask

[Chao Peng]

On Thu, May 28, 2015 at 01:38:05PM +0100, Jan Beulich wrote:
> >>> On 21.05.15 at 10:41,  wrote:
> > --- a/xen/arch/x86/mpparse.c
> > +++ b/xen/arch/x86/mpparse.c
> > @@ -87,6 +87,18 @@ void __init set_nr_cpu_ids(unsigned int max_cpus)
> >  #endif
> >  }
> >  
> > +void __init set_nr_sockets(void)
> > +{
> > +unsigned int cpus = bitmap_weight(phys_cpu_present_map.mask,
> > +  boot_cpu_data.x86_max_cores *
> > +  boot_cpu_data.x86_num_siblings);
> 
> How did you come to this expression for the bitmap size? I.e.
> why not simply physids_weight(phys_cpu_present_map)?

physids_weight(phys_cpu_present_map) gives me cpus for all sockets.
While here the 'cpus' is actually _cpus_per_socket_. I used the max
possible cpus indicated in cpuid as the upper bound so bitmap_weight()
returns the actual available cpus on socket 0.

> 
> > +
> > +if ( cpus == 0 )
> > +cpus = 1;
> > +
> > +nr_sockets = DIV_ROUND_UP(num_processors + disabled_cpus, cpus);
> > +}
> 
> Is there a reason why this can't just be added to the end of the
> immediately preceding set_nr_cpu_ids()?

You mean the declaration or invocation? If the former I have no special
reason for it (e.g. I can change it).

> > +/* Representing HT and core siblings in each socket */
> > +extern cpumask_var_t *socket_cpumask;
> 
> Comment style.

Ah, stop is missing here.

Chao

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

[Xen-devel] [linux-linus test] 57442: tolerable trouble: broken/fail/pass - PUSHED

[osstest service user]

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

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-libvirt  3 host-install(3) broken REGR. vs. 57123
 test-amd64-i386-libvirt  11 guest-start  fail   like 57123
 test-amd64-amd64-libvirt 11 guest-start  fail   like 57123
 test-amd64-i386-freebsd10-amd64  9 freebsd-install fail like 57123
 test-amd64-i386-freebsd10-i386  9 freebsd-install  fail like 57123
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 57123
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 57123

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 9 debian-hvm-install fail never 
pass
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 9 debian-hvm-install fail never 
pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail never 
pass
 test-amd64-i386-libvirt-xsm  11 guest-start  fail   never pass
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail never 
pass
 test-amd64-amd64-xl-xsm  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-libvirt-xsm 11 guest-start  fail   never pass
 test-amd64-i386-xl-xsm   11 guest-start  fail   never pass
 test-armhf-armhf-xl-xsm  11 guest-start  fail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-sedf 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail never pass
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail never pass
 test-armhf-armhf-libvirt-xsm 11 guest-start  fail   never pass
 test-armhf-armhf-xl-sedf-pin 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass

version targeted for testing:
 linuxde182468d1bb726198abaab315820542425270b7
baseline version:
 linuxc5db6a3bdeb72f4238e1faefa4ce4eab7a64baea


People who touched revisions under test:
  Alex Dowad 
  Alexei Starovoitov 
  Alexey Kardashevskiy 
  Aravind Gopalakrishnan 
  Avri Altman 
  Bartlomiej Zolnierkiewicz 
  Benjamin Poirier 
  Boris Brezillon 
  Borislav Petkov 
  Carl Schaefer 
  Carlo Caione 
  Chen Gang 
  Chen Gang 
  Chengyu Song 
  Cong Wang 
  Dan Carpenter 
  Daniel Borkmann 
  David S. Miller 
  David Vrabel 
  Dick Kennedy 
  Eliad Peller 
  Eliad Peller 
  Emmanuel Grumbach 
  Eric Dumazet 
  Federico Sauter 
  Florian Fainelli 
  Geert Uytterhoeven 
  Haim Dreyfuss 
  Hannes Frederic Sowa 
  Hannes Reinecke 
  Helge Deller 
  Herbert Xu 
  Ingo Molnar 
  Ivan Mikhaylov 
  James Bottomley 
  James Smart 
  James Smart 
  Jason Gunthorpe 
  Jayamohan Kallickal 
  Jeff Layton 
  Jiang Liu 
  Johan Hovold 
  Johannes Berg 
  Jordi Pujol Palomer 
  K. Y. Srinivasan 
  Kalle Valo 
  Lee Jones 
  Lendacky, Thomas 
  Liad Kaufman 
  Linus Torvalds 
  Linus Walleij 
  Luciano Coelho 
  Mark Hounschell 
  Mark Salyzyn 
  Mathieu Olivari 
  Matt Turner 
  Matti Gottlieb 
  Michael Brunner 
  Michael Brunner 
  Michal Kazior 
  Mika Westerberg 
  Miklos Szeredi 
  Minh Tran 
  Minh Tran 
  Nakajima Akira 
  Neil Horman 
  Nicolas Ferre 
  Paul Gortmaker 
  Rafał Miłecki 
  Ray Jui 
  Richard Cochran 
  Richard Henderson 
  Ross Lagerwall 
  Rusty Russell 
  Sachin Prabhu 
  Steve French 
  Thierry Reding 
  Tom Lendacky 
  Vladimir Zapolskiy 
  WANG Cong 
  Wei Liu 
  Yijing Wang 


jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-armhf-pvops  

Re: [Xen-devel] [PATCH] Documentation: extend use case for EXPORT_SYMBOL_GPL()

[Rob Landley]

On Thu, May 28, 2015 at 1:56 PM, Luis R. Rodriguez
 wrote:
> From: "Luis R. Rodriguez" 
>
> Current documentation over use case for EXPORT_SYMBOL_GPL()
> only acknowledges functions which are "an internal implementation
> issue, and not really an interface".

I.E. a statement of intent that this symbol is not intended as a
stable API that external users can program to regardless of underlying
implementation, and thus should not be viewed as a barrier to "derived
work" status for copyright purposes.

(If you get a book out of the library and program to its APIs, you're
not a derived work of an implementation not contained in that book.)

> In practice these days
> though we have some maintainers taking on preferences to require
> all new functionality go in with EXPORT_SYMBOL_GPL().

Ah, so they're predicting that Oracle will win the new "Apple vs
Franklin" case extending copyright to render whole new areas of
software proprietary, I.E. that APIs will be declared copyrightable
putting Wine and Samba and such out of business, and probably LEON's
ability to clone Sparc, and so on.

https://lwn.net/Articles/646160/

(And of course that TPP, which is now fast-tracked, will let us impose
this upon the rest of the world without appeal.)

Clearly, you want us to be on the side of Oracle, because GPL is
Freedom by definition no matter what the side effects everywhere else.

Bravo.

Rob

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

Re: [Xen-devel] [RFC][v2][PATCH 05/14] xen/x86/p2m: introduce set_identity_p2m_entry

[Chen, Tiejun]

On 2015/5/28 20:27, Jan Beulich wrote:

On 22.05.15 at 11:35,  wrote:

--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -898,6 +898,36 @@ int set_mmio_p2m_entry(struct domain *d, unsigned long 
gfn, mfn_t mfn,
  return set_typed_p2m_entry(d, gfn, mfn, p2m_mmio_direct, access);
  }

+int set_identity_p2m_entry(struct domain *d, unsigned long gfn,
+   p2m_access_t p2ma)
+{
+p2m_type_t p2mt;
+p2m_access_t a;
+mfn_t mfn;
+struct p2m_domain *p2m = p2m_get_hostp2m(d);
+int ret = -EBUSY;
+
+gfn_lock(p2m, gfn, 0);
+
+mfn = p2m->get_entry(p2m, gfn, &p2mt, &a, 0, NULL);
+
+if ( p2mt == p2m_invalid )
+ret = p2m_set_entry(p2m, gfn, _mfn(gfn), PAGE_ORDER_4K,
+p2m_mmio_direct, p2ma);
+else if ( mfn_x(mfn) == gfn && p2mt == p2m_mmio_direct && a == p2ma )
+ret = 0;
+else
+{
+printk(XENLOG_G_WARNING
+   "Cannot identity map d%d:%lx, already mapped to %lx.\n",
+   d->domain_id, gfn, mfn_x(mfn));
+}


With the redundant braces here dropped or the ret = -EBUSY moved
into this block,


Okay, I will fix this with the latter.


Reviewed-by: Jan Beulich 



Really thanks for your review.


I also reduced the Cc list quite significantly - I don't understand why
so many people were Cc-ed on this patch.



I just pick up all guys involving that design we posted previously to 
make sure they also pay attention on this series.


Thanks
Tiejun


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

Re: [Xen-devel] [PATCH v6 1/4] pci: add pci_iomap_wc() variants

[Luis R. Rodriguez]

On Wed, May 27, 2015 at 1:04 PM, Luis R. Rodriguez  wrote:
> On Tue, May 26, 2015 at 12:40:08PM -0500, Bjorn Helgaas wrote:
>> On Fri, May 22, 2015 at 02:23:41AM +0200, Luis R. Rodriguez wrote:
>> > On Thu, May 21, 2015 at 05:33:21PM -0500, Bjorn Helgaas wrote:
>> > >
>> > > I tentatively put this (and the rest of the series) on a pci/resource
>> > > branch.  I'm hoping you'll propose some clarification about
>> > > EXPORT_SYMBOL_GPL().
>> >
>> > EXPORT_SYMBOL_GPL() also serves to ensure only GPL modules can
>> > only run that code. So for instance although we have "Dual BSD/GPL"
>> > tags for modules pure "BSD" tags do not exist for module tags and
>> > cannot run EXPORT_SYMBOL_GPL() code [0]. Also there is some folks
>> > who do believe tha at run time all kernel modules are GPL [1] [2].
>> > And to be precise even though the FSF may claim a list of licenses
>> > are GPL-compatible we cannot rely on this list alone for our own
>> > goals and if folks want to use our EXPORT_SYMBOL_GPL()s they must
>> > discuss this on lkml [2].
>>
>> By "propose some clarification," I meant that I hoped you would propose a
>> patch to Documentation/ that would give maintainers some guidance.
>
> I *really really* would hate to do so but only because you insist, I'll look
> into this...

OK done. Please let me know if there is anything else I can do to
help. Also as per review with Tomi, the framebuffer maintainer, he
would prefer for only the required symbols to go through your tree.
We'd then wait for the next merge window for them to perculate to
Linus' tree and once there I'd send him a pull request for the
framebuffer device driver changes alone. So this does mean we'll have
no users of the symbols for a full release, but again, this is as per
Tomi's preference. This strategy is also the preference then for the
pci_iomap_wc() series as well. With that in mind, perhaps the lib
patch can go in as we'd have no users but we do have a few future
possible expected users.

 Luis

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

Re: [Xen-devel] [PATCH] Documentation: extend use case for EXPORT_SYMBOL_GPL()

[Luis R. Rodriguez]

On Thu, May 28, 2015 at 10:56:19PM +0100, Al Viro wrote:
> On Thu, May 28, 2015 at 11:17:36PM +0200, Luis R. Rodriguez wrote:
> > > ... while some of us consider that as pointless posturing and will refuse
> > > to merge such exports regardless.
> > 
> > Can you elaborate why, for those maintainers not aware of such positions?
> 
> *shrug*
> 
> Either one states that all modules are derivative works of the kernel,
> period (in which case attaching _GPL to specific exports is completely
> pointless), or it's a claim that this specific export is something
> special on its own, which is a fairly strong claim, completely unfounded
> more often than not.  In the worst cases it's the former being misrepresented
> as the latter.  That only serves to weaken our position in case of copyright
> violations, IMO.  When obviously BS claims like "encoding and decoding
> of UIDs between the numeric values as seen by userland and stored on
> filesystem and opaque pointers as used by the userns stuff is so special
> that its use alone is sufficient to change whether the code is derivative
> of the kernel or not" are thrown around, we end up with weaker protection,
> not stronger one.  If something like _that_ makes the difference between
> derived and non-derived, the former can't be worth much...

Great, thanks. This seems to be in alignment with those who have all along said
they've used EXPORT_SYMBOL() to mean what EXPORT_SYMBOL_GPL() users now use it
for. Nevertheless -- maintainers should know that some stubborn developers use
EXPORT_SYMBOL_GPL() for its technical merit should violators abuse those
symbols.

  Luis

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

[Xen-devel] [linux-3.4 test] 57436: regressions - FAIL

[osstest service user]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl   9 debian-install fail REGR. vs. 52209-bisect
 test-amd64-amd64-pair   15 debian-install/dst_host fail REGR. vs. 52715-bisect
 test-amd64-i386-pair15 debian-install/dst_host fail REGR. vs. 56366-bisect

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-sedf  9 debian-installfail blocked in 56366-bisect
 test-amd64-amd64-libvirt  9 debian-installfail blocked in 56366-bisect
 test-amd64-amd64-xl-qemut-debianhvm-amd64 6 xen-boot fail blocked in 
56366-bisect
 test-amd64-i386-libvirt   9 debian-installfail blocked in 56366-bisect
 test-amd64-i386-qemut-rhel6hvm-intel  6 xen-boot  fail blocked in 56366-bisect
 test-amd64-i386-xl-qemuu-ovmf-amd64  6 xen-boot   fail blocked in 56366-bisect
 test-amd64-i386-freebsd10-amd64  6 xen-boot   fail blocked in 56366-bisect
 test-amd64-amd64-xl-multivcpu  6 xen-boot fail blocked in 56366-bisect
 test-amd64-i386-libvirt-xsm   6 xen-boot  fail blocked in 56366-bisect
 test-amd64-i386-xl9 debian-installfail blocked in 56366-bisect
 test-amd64-i386-rumpuserxen-i386  6 xen-boot  fail blocked in 56366-bisect
 test-amd64-i386-xl-qemuu-winxpsp3  6 xen-boot fail blocked in 56366-bisect
 test-amd64-amd64-xl-sedf-pin  9 debian-installfail blocked in 56366-bisect
 test-amd64-i386-xl-qemut-debianhvm-amd64 6 xen-boot fail blocked in 
56366-bisect
 test-amd64-amd64-rumpuserxen-amd64  6 xen-bootfail blocked in 56366-bisect
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 6 xen-boot fail blocked in 
56366-bisect
 test-amd64-i386-xl-qemuu-debianhvm-amd64 6 xen-boot fail blocked in 
56366-bisect
 test-amd64-i386-qemuu-rhel6hvm-intel  6 xen-boot  fail blocked in 56366-bisect
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail blocked in 56366-bisect
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail blocked in 56366-bisect
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail blocked in 56366-bisect
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 6 xen-boot fail blocked in 
56366-bisect
 test-amd64-i386-xl-qemut-winxpsp3-vcpus1 6 xen-boot fail blocked in 
56366-bisect
 test-amd64-i386-freebsd10-i386  6 xen-bootfail blocked in 56366-bisect
 test-amd64-amd64-xl-qemuu-winxpsp3  6 xen-bootfail blocked in 56366-bisect
 test-amd64-i386-xl-qemut-winxpsp3  6 xen-boot fail blocked in 56366-bisect
 test-amd64-amd64-xl-qemut-winxpsp3  6 xen-bootfail blocked in 56366-bisect
 test-amd64-amd64-xl-qemuu-ovmf-amd64  6 xen-bootfail like 53709-bisect

Tests which did not succeed, but are not blocking:
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 9 debian-hvm-install fail never 
pass
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail never 
pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail never 
pass
 test-amd64-i386-xl-xsm9 debian-install   fail   never pass
 test-amd64-amd64-xl-xsm   9 debian-install   fail   never pass
 test-amd64-amd64-libvirt-xsm  9 debian-install   fail   never pass
 test-amd64-amd64-xl-credit2   9 debian-install   fail   never pass
 test-amd64-amd64-xl-pvh-intel  9 debian-install   fail  never pass
 test-amd64-amd64-xl-pvh-amd   9 debian-install   fail   never pass
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 9 debian-hvm-install fail never 
pass
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail never pass

version targeted for testing:
 linux56b48fcda5076d4070ab00df32ff5ff834e0be86
baseline version:
 linuxbb4a05a0400ed6d2f1e13d1f82f289ff74300a70


370 people touched revisions under test,
not listing them all


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
 build-amd64-rumpuserxen  pass
 build-i386-rumpuserxen   pass
 test-amd64-amd64-xl  fail
 test-amd64-i386-xl   fail
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsmfail
 

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

[osstest service user]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail REGR. vs. 56492
 test-amd64-i386-xl-qemuu-win7-amd64  9 windows-installfail REGR. vs. 56492

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail never 
pass
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail never 
pass

version targeted for testing:
 ovmf b5ec7f3c329232497ef7894c0d64fc996dba8a55
baseline version:
 ovmf f1f0f0deb6e64df6b7c04ead7330afecf5537e46


People who touched revisions under test:
  "Mudusuru, Giri P" 
  "Yao, Jiewen" 
  Chao Zhang 
  Dandan Bi 
  Eric Dong 
  Feng Tian 
  Guo Dong 
  Hao Wu 
  Heyi Guo 
  Jeff Fan 
  jiaxinwu 
  Liming Gao 
  Ludovic Rousseau 
  Mudusuru, Giri P 
  Olivier Martin 
  Ruiyu Ni 
  Shifei Lu 
  Star Zeng 
  Yao, Jiewen 
  Yingke Liu 


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-debianhvm-amd64-xsmfail
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm fail
 test-amd64-i386-qemuu-rhel6hvm-amd   pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-i386-xl-qemuu-debianhvm-amd64 pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  pass
 test-amd64-amd64-xl-qemuu-win7-amd64 fail
 test-amd64-i386-xl-qemuu-win7-amd64  fail
 test-amd64-i386-qemuu-rhel6hvm-intel pass
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 pass
 test-amd64-amd64-xl-qemuu-winxpsp3   pass
 test-amd64-i386-xl-qemuu-winxpsp3pass



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

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 993 lines long.)

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

Re: [Xen-devel] [PATCH] Documentation: extend use case for EXPORT_SYMBOL_GPL()

[Al Viro]

On Thu, May 28, 2015 at 11:17:36PM +0200, Luis R. Rodriguez wrote:
> > ... while some of us consider that as pointless posturing and will refuse
> > to merge such exports regardless.
> 
> Can you elaborate why, for those maintainers not aware of such positions?

*shrug*

Either one states that all modules are derivative works of the kernel,
period (in which case attaching _GPL to specific exports is completely
pointless), or it's a claim that this specific export is something
special on its own, which is a fairly strong claim, completely unfounded
more often than not.  In the worst cases it's the former being misrepresented
as the latter.  That only serves to weaken our position in case of copyright
violations, IMO.  When obviously BS claims like "encoding and decoding
of UIDs between the numeric values as seen by userland and stored on
filesystem and opaque pointers as used by the userns stuff is so special
that its use alone is sufficient to change whether the code is derivative
of the kernel or not" are thrown around, we end up with weaker protection,
not stronger one.  If something like _that_ makes the difference between
derived and non-derived, the former can't be worth much...

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

Re: [Xen-devel] [PATCH v6 0/2] kconfig: add xenconfig

[Luis R. Rodriguez]

On Thu, May 21, 2015 at 11:32 AM, Luis R. Rodriguez  wrote:
> On Thu, May 21, 2015 at 04:20:27PM +0800, Michal Marek wrote:
>> Dne 21.5.2015 v 02:53 Luis R. Rodriguez napsal(a):
>> > From: "Luis R. Rodriguez" 
>> >
>> > Michal Marek, Xen folks (David Vrabel, Konrad, Ian), which tree should
>> > these go through?
>>
>> Not kbuild, if I may ask :). Otherwise people will find me in
>> get_maintainer.pl output and keep CCing me on updates to the xen.config
>> files, which is something I am not at all familiar with.
>
> As you wish :)

David, Konrad, Ian,

Michal prefers this to go through the Xen tree. Please let me know if
there any issues with the patches.

 Luis

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

Re: [Xen-devel] [PATCH] Documentation: extend use case for EXPORT_SYMBOL_GPL()

[Luis R. Rodriguez]

On Thu, May 28, 2015 at 09:07:49PM +0100, Al Viro wrote:
> On Thu, May 28, 2015 at 11:56:01AM -0700, Luis R. Rodriguez wrote:
> > From: "Luis R. Rodriguez" 
> > 
> > Current documentation over use case for EXPORT_SYMBOL_GPL()
> > only acknowledges functions which are "an internal implementation
> > issue, and not really an interface". In practice these days
> > though we have some maintainers taking on preferences to require
> > all new functionality go in with EXPORT_SYMBOL_GPL().
> > 
> > A maintainer asking developers to use EXPORT_SYMBOL_GPL()
> > for new functionality tends to be a well accepted and understood
> > position that maintainers can take and typically requires the
> > maintainers educating contributing developers on their own
> > positions and requirements.
> > 
> > Developers who submit code to maintainers not familiar with
> > these preferences as optional for new functionality need explicit
> > guidence though as existing documentation does not acknowledge
> > this as a valid possibility. Without this being documented some
> > maintainers are reluctant to accept new functionality with
> > EXPORT_SYMBOL_GPL().
> > 
> > This extends the use case documentation for EXPORT_SYMBOL_GPL()
> > to acknowledge acceptance for new functionality.
>  
> ... while some of us consider that as pointless posturing and will refuse
> to merge such exports regardless.

Can you elaborate why, for those maintainers not aware of such positions?

> It's _NOT_ a universal default; please, do not attempt to imply otherwise.

Indeed, the documentation does not try to do that.

>  In particular, for fs/*.c I will not
> accept that as valid grounds for use of EXPORT_SYMBOL_GPL.

Understood.

  Luis

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

[Xen-devel] [libvirt test] 57444: regressions - FAIL

[osstest service user]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-libvirt  11 guest-start   fail REGR. vs. 53854
 test-amd64-amd64-libvirt 11 guest-start   fail REGR. vs. 53854
 test-armhf-armhf-libvirt 11 guest-start   fail REGR. vs. 53854

Tests which did not succeed, but are not blocking:
 test-amd64-i386-libvirt-xsm  11 guest-start  fail   never pass
 test-amd64-amd64-libvirt-xsm 11 guest-start  fail   never pass
 test-armhf-armhf-libvirt-xsm 11 guest-start  fail   never pass

version targeted for testing:
 libvirt  efc68de5cd70d62b1941a707d22299422f1962f9
baseline version:
 libvirt  fd74e231751334b64af0934b680c5cc62f652453


People who touched revisions under test:
  Andrea Bolognani 
  Boris Fiuczynski 
  Cole Robinson 
  Daniel Hansel 
  Daniel P. Berrange 
  Eric Blake 
  Erik Skultety 
  Jim Fehlig 
  Jiri Denemark 
  John Ferlan 
  Ján Tomko 
  Laine Stump 
  Lubomir Rintel 
  Luyao Huang 
  Martin Kletzander 
  Maxim Nestratov 
  Michal Privoznik 
  Nikolay Shirokovskiy 
  Pavel Fedin 
  Pavel Hrdina 
  Peter Krempa 
  Richard W.M. Jones 
  Roman Bogorodskiy 
  Tony Krowiak 
  Tony Krowiak 
  Viktor Mihajlovski 
  Wang Yufei 
  Zhang Bo 


jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-xsm fail
 test-armhf-armhf-libvirt-xsm fail
 test-amd64-i386-libvirt-xsm  fail
 test-amd64-amd64-libvirt fail
 test-armhf-armhf-libvirt fail
 test-amd64-i386-libvirt  fail



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

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

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 1869 lines long.)

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

[Xen-devel] [PATCH] frontswap: allow multiple backends

[Dan Streetman]

Change frontswap single pointer to a singly linked list of frontswap
implementations.  Update Xen tmem implementation as register no longer
returns anything.

Frontswap only keeps track of a single implementation; any implementation
that registers second (or later) will replace the previously registered
implementation, and gets a pointer to the previous implementation that
the new implementation is expected to pass all frontswap functions to
if it can't handle the function itself.  However that method doesn't
really make much sense, as passing that work on to every implementation
adds unnecessary work to implementations; instead, frontswap should
simply keep a list of all registered implementations and try each
implementation for any function.  Most importantly, neither of the
two currently existing frontswap implementations in the kernel actually
do anything with any previous frontswap implementation that they
replace when registering.

This allows frontswap to successfully manage multiple implementations
by keeping a list of them all.

Signed-off-by: Dan Streetman 
---
 drivers/xen/tmem.c|   8 +--
 include/linux/frontswap.h |   4 +-
 mm/frontswap.c| 159 +-
 3 files changed, 88 insertions(+), 83 deletions(-)

diff --git a/drivers/xen/tmem.c b/drivers/xen/tmem.c
index c4211a3..d88f367 100644
--- a/drivers/xen/tmem.c
+++ b/drivers/xen/tmem.c
@@ -381,15 +381,9 @@ static int __init xen_tmem_init(void)
 #ifdef CONFIG_FRONTSWAP
if (tmem_enabled && frontswap) {
char *s = "";
-   struct frontswap_ops *old_ops;
 
tmem_frontswap_poolid = -1;
-   old_ops = frontswap_register_ops(&tmem_frontswap_ops);
-   if (IS_ERR(old_ops) || old_ops) {
-   if (IS_ERR(old_ops))
-   return PTR_ERR(old_ops);
-   s = " (WARNING: frontswap_ops overridden)";
-   }
+   frontswap_register_ops(&tmem_frontswap_ops);
pr_info("frontswap enabled, RAM provided by Xen Transcendent 
Memory%s\n",
s);
}
diff --git a/include/linux/frontswap.h b/include/linux/frontswap.h
index 8293262..9b3fc53 100644
--- a/include/linux/frontswap.h
+++ b/include/linux/frontswap.h
@@ -11,11 +11,11 @@ struct frontswap_ops {
int (*load)(unsigned, pgoff_t, struct page *);
void (*invalidate_page)(unsigned, pgoff_t);
void (*invalidate_area)(unsigned);
+   struct frontswap_ops *next;
 };
 
 extern bool frontswap_enabled;
-extern struct frontswap_ops *
-   frontswap_register_ops(struct frontswap_ops *ops);
+extern void frontswap_register_ops(struct frontswap_ops *ops);
 extern void frontswap_shrink(unsigned long);
 extern unsigned long frontswap_curr_pages(void);
 extern void frontswap_writethrough(bool);
diff --git a/mm/frontswap.c b/mm/frontswap.c
index 8d82809..46f21f8 100644
--- a/mm/frontswap.c
+++ b/mm/frontswap.c
@@ -21,8 +21,8 @@
 #include 
 
 /*
- * frontswap_ops is set by frontswap_register_ops to contain the pointers
- * to the frontswap "backend" implementation functions.
+ * frontswap_ops are added by frontswap_register_ops, and provide the
+ * frontswap "backend" implementation functions.
  */
 static struct frontswap_ops *frontswap_ops __read_mostly;
 
@@ -79,15 +79,6 @@ static inline void inc_frontswap_invalidates(void) { }
  * on all frontswap functions to not call the backend until the backend
  * has registered.
  *
- * Specifically when no backend is registered (nobody called
- * frontswap_register_ops) all calls to frontswap_init (which is done via
- * swapon -> enable_swap_info -> frontswap_init) are registered and remembered
- * (via the setting of need_init bitmap) but fail to create tmem_pools. When a
- * backend registers with frontswap at some later point the previous
- * calls to frontswap_init are executed (by iterating over the need_init
- * bitmap) to create tmem_pools and set the respective poolids. All of that is
- * guarded by us using atomic bit operations on the 'need_init' bitmap.
- *
  * This would not guards us against the user deciding to call swapoff right as
  * we are calling the backend to initialize (so swapon is in action).
  * Fortunatly for us, the swapon_mutex has been taked by the callee so we are
@@ -106,37 +97,51 @@ static inline void inc_frontswap_invalidates(void) { }
  *
  * Obviously the opposite (unloading the backend) must be done after all
  * the frontswap_[store|load|invalidate_area|invalidate_page] start
- * ignorning or failing the requests - at which point frontswap_ops
- * would have to be made in some fashion atomic.
+ * ignorning or failing the requests.  However, there is currently no way
+ * to unload a backend once it is registered.
  */
-static DECLARE_BITMAP(need_init, MAX_SWAPFILES);
 
 /*
- * Register operations for frontswap, returning previous thus allowing
- * detection of multiple backends an

Re: [Xen-devel] [PATCH] Documentation: extend use case for EXPORT_SYMBOL_GPL()

[Al Viro]

On Thu, May 28, 2015 at 11:56:01AM -0700, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" 
> 
> Current documentation over use case for EXPORT_SYMBOL_GPL()
> only acknowledges functions which are "an internal implementation
> issue, and not really an interface". In practice these days
> though we have some maintainers taking on preferences to require
> all new functionality go in with EXPORT_SYMBOL_GPL().
> 
> A maintainer asking developers to use EXPORT_SYMBOL_GPL()
> for new functionality tends to be a well accepted and understood
> position that maintainers can take and typically requires the
> maintainers educating contributing developers on their own
> positions and requirements.
> 
> Developers who submit code to maintainers not familiar with
> these preferences as optional for new functionality need explicit
> guidence though as existing documentation does not acknowledge
> this as a valid possibility. Without this being documented some
> maintainers are reluctant to accept new functionality with
> EXPORT_SYMBOL_GPL().
> 
> This extends the use case documentation for EXPORT_SYMBOL_GPL()
> to acknowledge acceptance for new functionality.
 
... while some of us consider that as pointless posturing and will refuse
to merge such exports regardless.  It's _NOT_ a universal default; please,
do not attempt to imply otherwise.  In particular, for fs/*.c I will not
accept that as valid grounds for use of EXPORT_SYMBOL_GPL.

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

Re: [Xen-devel] [PATCH] Documentation: extend use case for EXPORT_SYMBOL_GPL()

[Luis R. Rodriguez]

On Thu, May 28, 2015 at 01:09:23PM -0600, Jonathan Corbet wrote:
> On Thu, 28 May 2015 11:56:01 -0700
> "Luis R. Rodriguez"  wrote:
> 
> > +Some maintainers and developers may however have a preference to
> > +require EXPORT_SYMBOL_GPL() when adding any new APIs or functionality.
> 
> As a nit, I would take out "have a preference to".

Fine by me, do you need a new submission on my part of can you amend yourself?

> From what I can tell, there are developers who think this position makes
> little sense and, perhaps, risks diluting the value of EXPORT_SYMBOL_GPL()
> by attaching it to everything.  My inclination, though, would be to accept
> this change as documentation of clear existing practice; whether that
> practice should change is, I think, a separate discussion.

Great, thanks.

  Luis

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

[Xen-devel] [linux-3.18 test] 57420: regressions - trouble: broken/fail/pass

[osstest service user]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf  3 host-install(3) broken in 57345 REGR. vs. 57312
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail REGR. vs. 57312

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-libvirt-xsm  3 host-install(3)  broken in 57378 pass in 57420
 test-armhf-armhf-libvirt  3 host-install(3)  broken in 57378 pass in 57420
 test-armhf-armhf-xl   3 host-install(3)   broken pass in 57378
 test-armhf-armhf-xl-cubietruck  3 host-install(3) broken pass in 57378
 test-armhf-armhf-xl-sedf-pin  6 xen-boot   fail in 57378 pass in 57420
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail in 57378 pass in 57420
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop  fail pass in 57345
 test-amd64-amd64-xl-sedf 13 guest-saverestore   fail pass in 57378

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-credit2   3 host-install(3)  broken like 57312
 test-armhf-armhf-xl-xsm   3 host-install(3)  broken like 57312
 test-armhf-armhf-xl-sedf  6 xen-boot  fail REGR. vs. 57312

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-sedf  1 build-check(1)blocked in 57345 n/a
 test-armhf-armhf-xl-sedf-pin  1 build-check(1)blocked in 57345 n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)blocked in 57345 n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked in 57345 n/a
 build-armhf-libvirt   1 build-check(1)blocked in 57345 n/a
 test-armhf-armhf-libvirt-xsm  1 build-check(1)blocked in 57345 n/a
 test-armhf-armhf-libvirt  1 build-check(1)blocked in 57345 n/a
 test-armhf-armhf-xl   1 build-check(1)blocked in 57345 n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)blocked in 57345 n/a
 test-armhf-armhf-xl-cubietruck  1 build-check(1)  blocked in 57345 n/a
 test-armhf-armhf-xl   6 xen-boot  fail in 57378 never pass
 test-armhf-armhf-xl-credit2   6 xen-boot  fail in 57378 never pass
 test-armhf-armhf-xl-cubietruck  6 xen-bootfail in 57378 never pass
 test-armhf-armhf-xl-xsm   6 xen-boot  fail in 57378 never pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail never 
pass
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 9 debian-hvm-install fail never 
pass
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail never 
pass
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 9 debian-hvm-install fail never 
pass
 test-amd64-i386-xl-xsm   11 guest-start  fail   never pass
 test-amd64-amd64-libvirt-xsm 11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-i386-libvirt-xsm  11 guest-start  fail   never pass
 test-amd64-amd64-xl-xsm  11 guest-start  fail   never pass
 test-amd64-i386-freebsd10-i386  9 freebsd-install  fail never pass
 test-amd64-i386-freebsd10-amd64  9 freebsd-install fail never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-sedf-pin 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail never pass
 test-armhf-armhf-libvirt-xsm  6 xen-boot fail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass

version targeted for testing:
 linux51af817611f2c0987030d024f24fc7ea95dd33e6
baseline version:
 linuxb2776bf7149bddd1f4161f14f79520f17fc1d71d


900 people touched revisions under test,
not listing them all


jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  p

[Xen-devel] [PATCH] Documentation: extend use case for EXPORT_SYMBOL_GPL()

[Luis R. Rodriguez]

From: "Luis R. Rodriguez" 

Current documentation over use case for EXPORT_SYMBOL_GPL()
only acknowledges functions which are "an internal implementation
issue, and not really an interface". In practice these days
though we have some maintainers taking on preferences to require
all new functionality go in with EXPORT_SYMBOL_GPL().

A maintainer asking developers to use EXPORT_SYMBOL_GPL()
for new functionality tends to be a well accepted and understood
position that maintainers can take and typically requires the
maintainers educating contributing developers on their own
positions and requirements.

Developers who submit code to maintainers not familiar with
these preferences as optional for new functionality need explicit
guidence though as existing documentation does not acknowledge
this as a valid possibility. Without this being documented some
maintainers are reluctant to accept new functionality with
EXPORT_SYMBOL_GPL().

This extends the use case documentation for EXPORT_SYMBOL_GPL()
to acknowledge acceptance for new functionality.

Cc: Jonathan Corbet 
Cc: linux-...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Luis R. Rodriguez 
---
 Documentation/DocBook/kernel-hacking.tmpl | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/DocBook/kernel-hacking.tmpl 
b/Documentation/DocBook/kernel-hacking.tmpl
index e84f094..19e0c88 100644
--- a/Documentation/DocBook/kernel-hacking.tmpl
+++ b/Documentation/DocBook/kernel-hacking.tmpl
@@ -954,6 +954,8 @@ printk(KERN_INFO "my ip: %pI4\n", &ipaddress);
 MODULE_LICENSE() that specifies a GPL
 compatible license.  It implies that the function is considered
 an internal implementation issue, and not really an interface.
+Some maintainers and developers may however have a preference to
+require EXPORT_SYMBOL_GPL() when adding any new APIs or functionality.

   
  
-- 
2.3.2.209.gd67f9d5.dirty


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

Re: [Xen-devel] [libvirt] [PATCH 0/4] libxl: support SPICE graphics

[Jim Fehlig]

On 05/28/2015 05:10 AM, Michal Privoznik wrote:

On 09.05.2015 00:31, Jim Fehlig wrote:

This series provides support for SPICE graphics in the libxl driver.
The first patch is mostly code movement.  The second patch is a trivial
name change of a structure member.  Patch3 populates the
libxl_domain_build_info struct with SPICE info.  SPICE isn't as nice
without QXL, so patch4 provides support for QXL video device.

Jim Fehlig (4):
   libxl: populate build_info vfb in separate function
   libxl: change reservedVNCPorts to reservedGraphicsPorts
   libxl: support SPICE graphics for HVM domains
   libxl: support QXL video device

  src/libxl/libxl_conf.c   | 150 +--
  src/libxl/libxl_conf.h   |   2 +-
  src/libxl/libxl_domain.c |   8 ++-
  src/libxl/libxl_driver.c |   4 +-
  4 files changed, 128 insertions(+), 36 deletions(-)


ACK to all, but please see my comment to 3/4 before pushing.

Even though we are in freeze, this has been lying around for a while
therefore I think it's safe to push these.


Thanks for the review and finding the bug in 3/4!  I changed it as we discussed 
and pushed the series.


Regards,
Jim


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

[Xen-devel] [xen-unstable test] 57419: tolerable FAIL - PUSHED

[osstest service user]

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

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-libvirt 11 guest-start   fail REGR. vs. 56375
 test-amd64-i386-freebsd10-amd64 19 guest-start/freebsd.repeat fail blocked in 
56375
 test-amd64-i386-libvirt-xsm  11 guest-start  fail   like 56375
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 56375
 test-amd64-i386-libvirt  11 guest-start  fail   like 56375
 test-armhf-armhf-libvirt 11 guest-start  fail   like 56375
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 56375
 test-amd64-amd64-rumpuserxen-amd64 15 
rumpuserxen-demo-xenstorels/xenstorels.repeat fail like 56544-bisect

Tests which did not succeed, but are not blocking:
 test-amd64-i386-xl-xsm   14 guest-localmigrate   fail   never pass
 test-amd64-amd64-xl-xsm  14 guest-localmigrate   fail   never pass
 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-amd64-xl-qemut-debianhvm-amd64-xsm 12 guest-localmigrate fail never 
pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 12 guest-localmigrate fail never 
pass
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 12 guest-localmigrate fail never 
pass
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 12 guest-localmigrate fail never 
pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail never pass
 test-armhf-armhf-libvirt-xsm 11 guest-start  fail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-sedf 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-sedf-pin 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail never pass

version targeted for testing:
 xen  d6b6bd8374ac30597495d457829ce7ad6e8b7016
baseline version:
 xen  e13013dbf1d5997915548a3b5f1c39594d8c1d7b


People who touched revisions under test:
  Andrew Cooper 
  Charles Arnold 
  Chen Baozi 
  Daniel De Graaf 
  David Vrabel 
  George Dunlap 
  Huaitong Han 
  Ian Campbell 
  Ian Jackson 
  Jan Beulich 
  Julien Grall 
  Julien Grall  (ARM)
  Julien Grall 
  Kevin Tian 
  Konrad Rzeszutek Wilk 
  Len Brown 
  Olaf Hering 
  Pranavkumar Sawargaonkar 
  Roger Pau Monné 
  Tiejun Chen 
  Tim Deegan 
  Wei Liu 


jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-oldkern  pass
 build-i386-oldkern   pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 build-amd64-rumpuserxen  pass
 build-i386-rumpuserxen   pass
 test-amd64-amd64-xl  pass
 test-armhf-armhf-xl  pass
 test-amd64-i386-xl   pass
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsmfail
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm fail
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsmfail
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm fail
 test-amd64-amd64-libvirt-xsm pass
 test-armhf-armhf-libvirt-xsm fail
 test-amd64-i386-libvirt-xsm  fail
 

Re: [Xen-devel] [PATCH V7] xen/vm_event: Clean up control-register-write vm_events and add XCR0 event

[Razvan Cojocaru]

On 05/28/2015 07:06 PM, Lengyel, Tamas wrote:
> 
> 
> On Thu, May 28, 2015 at 10:33 AM, Razvan Cojocaru
> mailto:rcojoc...@bitdefender.com>> wrote:
> 
> On 05/28/2015 05:12 PM, Lengyel, Tamas wrote:
> > diff --git a/xen/arch/x86/hvm/event.c b/xen/arch/x86/hvm/event.c
> > index 9d5f9f3..a13195d 100644
> > --- a/xen/arch/x86/hvm/event.c
> > +++ b/xen/arch/x86/hvm/event.c
> > @@ -21,6 +21,7 @@
> >
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >
> >  static void hvm_event_fill_regs(vm_event_request_t *req)
> > @@ -88,55 +89,28 @@ static int hvm_event_traps(uint8_t sync,
> > vm_event_request_t *req)
> >  return 1;
> >  }
> >
> > -static inline
> > -void hvm_event_cr(uint32_t reason, unsigned long value,
> > - unsigned long old, bool_t onchangeonly,
> > bool_t sync)
> > +void (hvm_event_cr)(unsigned int index, unsigned long value,
> > unsigned long old)
> >
> >
> > Any reason to have parenthesis around hvm_event_cr? Also, if you could
> > make the patch available somewhere via git, that would be great, I would
> > like to test it!
> 
> Yes, to prevent macro expansion. Jan has suggested that I add a macro to
> make the calls look neater, please see this change:
> 
> +void hvm_event_cr(unsigned int index, unsigned long value, unsigned
> long old);
> +#define hvm_event_cr(what, new, old) hvm_event_cr(VM_EVENT_X86_##what,
> new, old)
> 
> So, unless that call is put in parenthesis, the code you mention is
> expanded to hvm_event_cr(VM_EVENT_X86_unsigned int index, ...) or
> something of that nature, and it won't compile.
> 
> 
> That seems a bit odd to me that we now have both a macro and a function
> with the same name. I think making the macro at least all capital would
> avoid confusion and make the code a lot more readable. But even then
> IMHO this wrapper is not helping code readability much. Its really a
> pain to track down auto-expanding fields in macros when someone is
> trying to read the code.

I think the lowercase version of the wrapper macro of the same name is a
Xen coding style convention, but if Jan thinks going uppercase is not a
problem that's fine with me.

As for the wrapper, being right next to the function it is wrapping
makes it hard to ignore when reading the header, and the #define is
pretty clear on what it does.

Also, while I think that the macro does help with readability (so my
preference would be to leave it in), it's just that - a preference - so
if Jan also agrees that we should now remove it I'll do that. After all,
the macro will probably go out the window (or the first parameter will
need to be changed to VM_EVENT_##what instead of VM_EVENT_X86_##what) as
soon as ARM control register write events will come into play.

> As for a git repo, unfortunately I don't have a public one, but I intend
> to submit V8 shortly, and if I understood things correctly it's quite
> likely to enter staging, so it should only be a short while now.
> 
> 
> I guess but putting it up on github as well should really not be a problem..

True, I'll see what I can do about that in the future, but in the
meantime I'll send you the patch file privately (exported with "git
format-patch master") and you can just apply it with "git am" to the
current Xen source code.


Thanks,
Razvan


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

Re: [Xen-devel] [tools/libxl]: Possible bug in ao_cancel

[Ian Jackson]

Koushik Chakravarty writes ("[tools/libxl]: Possible bug in ao_cancel"):
> I spotted a possible bug in ao_cancel (libxl_event.c) and wanted to run it 
> through you.

Hi, sure.  (Sorry for the delay replying, I have been away.  And I'm
going away again for the whole of next week...)

> In the ao_cancel(), we mark the parent->cancelling = 1 so that subsequence 
> cancel calls don't get entertained and mess things up. However, in my view, 
> setting this should be after we check for "parent->cancellables".
> 
> This is because, if someones invokes libxl_ao_cancel(), while there are no 
> cancellables registered, then further calls to libxl_ao_cancel() should not 
> be rejected - as the first call actually didn't do anything.
> I hope I am making myself clear.

Yes, your question is clear, thanks.  I don't think you're right,
though.

After cancelling is set, libxl__ao_cancellable_register rejects any
further attempts to set up any cancellables.  So I think the situation
you describe (where the libxl_ao_cancel would do nothing but return
CANCELLED because cancelling was already set, but in fact there is a
cancellable which could be notified) cannot arise.

Thanks,
Ian.

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

Re: [Xen-devel] [PATCHv10 3/4] gnttab: make the grant table lock a read-write lock

[David Vrabel]

On 28/05/15 15:55, Jan Beulich wrote:
 On 26.05.15 at 20:00,  wrote:
>> @@ -254,23 +254,23 @@ double_gt_lock(struct grant_table *lgt, struct 
>> grant_table *rgt)
>>  {
>>  if ( lgt < rgt )
>>  {
>> -spin_lock(&lgt->lock);
>> -spin_lock(&rgt->lock);
>> +write_lock(&lgt->lock);
>> +write_lock(&rgt->lock);
>>  }
>>  else
>>  {
>>  if ( lgt != rgt )
>> -spin_lock(&rgt->lock);
>> -spin_lock(&lgt->lock);
>> +write_lock(&rgt->lock);
>> +write_lock(&lgt->lock);
>>  }
>>  }
> 
> So I looked at the two uses of double_gt_lock() again: in both cases
> only a read lock is needed on rgt (which is also the natural thing to
> expect: we aren't supposed to modify the remote domain's grant
> table in any way here). Albeit that's contradicting ...

See comment below.

>> @@ -568,10 +568,10 @@ static void mapcount(
>>  *wrc = *rdc = 0;
>>  
>>  /*
>> - * Must have the remote domain's grant table lock while counting
>> - * its active entries.
>> + * Must have the remote domain's grant table write lock while
>> + * counting its active entries.
>>   */
>> -ASSERT(spin_is_locked(&rd->grant_table->lock));
>> +ASSERT(rw_is_write_locked(&rd->grant_table->lock));
> 
> ... this: Why would we need to hold the write lock here? We're
> not changing anything in rd->grant_table.
> 
>> @@ -837,12 +838,22 @@ __gnttab_map_grant_ref(
>>  
>>  TRACE_1D(TRC_MEM_PAGE_GRANT_MAP, op->dom);
>>  
>> +/*
>> + * All maptrack entry users check mt->flags first before using the
>> + * other fields so just ensure the flags field is stored last.
>> + *
>> + * However, if gnttab_need_iommu_mapping() then this would race
>> + * with a concurrent mapcount() call (on an unmap, for example)
>> + * and a lock is required.
>> + */
>>  mt = &maptrack_entry(lgt, handle);
>>  mt->domid = op->dom;
>>  mt->ref   = op->ref;
>> -mt->flags = op->flags;
>> +wmb();
>> +write_atomic(&mt->flags, op->flags);
> 
> Doesn't that then also require the use of read_atomic() and rmb()
> on the reading side?

rmb() isn't required because the read_lock() is already a full barrier.
 Will need to add some read_atomic()s though.

> Further, why are only races against mapcount()
> a problem, but not such against __gnttab_unmap_common() as a
> whole? I.e. what's the locking around the op->map->flags and
> op->map->domid accesses below good for? Or, alternatively, isn't
> this an indication of a problem with the previous patch splitting off
> the maptrack lock (effectively leaving some map track entry
> accesses without any guard)?

The double_gt_lock() takes both write locks, thus does not race with
__gnttab_unmap_common clearing the flag on the maptrack entry which is
done while holding the remote read lock.

The maptrack lock only protects the free list, not the maptrack entries
themselves.  The docs may not correctly say this though -- i'll double
check.

>> -double_gt_unlock(lgt, rgt);
>> +if ( gnttab_need_iommu_mapping(ld) )
>> +double_gt_unlock(lgt, rgt);
> 
> I think you shouldn't rely on gnttab_need_iommu_mapping() to
> produce the same result upon this and the earlier invocation, i.e.
> you ought to latch the first call's result into a local variable.

Um. Okay. But if this changes during the life time of a domain it's
going to either leak iommu mappings or fail to create them which sounds
rather broken to me.

>> @@ -1787,7 +1805,7 @@ gnttab_transfer(
>>  TRACE_1D(TRC_MEM_PAGE_GRANT_TRANSFER, e->domain_id);
>>  
>>  /* Tell the guest about its new page frame. */
>> -spin_lock(&e->grant_table->lock);
>> +read_lock(&e->grant_table->lock);
>>  
>>  if ( e->grant_table->gt_version == 1 )
>>  {
>> @@ -1805,7 +1823,7 @@ gnttab_transfer(
>>  shared_entry_header(e->grant_table, gop.ref)->flags |=
>>  GTF_transfer_completed;
>>  
>> -spin_unlock(&e->grant_table->lock);
>> +read_unlock(&e->grant_table->lock);
> 
> I'm not sure a read lock suffices here: Consider the effect of two
> parallel invocations of this code with identical gop.ref, but different
> gop.mfn. Or wait, gnttab_prepare_for_transfer() makes sure only
> one gets here, unless the granting domain fiddles with the shared
> entry. But it feels wrong nevertheless, considering that we alter
> state of e here. Or should this perhaps lock the matching active
> entry, even if it doesn't touch it?

Locking the active entry looks sensible to me.

>> @@ -2645,7 +2663,7 @@ __gnttab_swap_grant_ref(grant_ref_t ref_a, grant_ref_t 
>> ref_b)
>>  struct active_grant_entry *act_b = NULL;
>>  s16 rc = GNTST_okay;
>>  
>> -spin_lock(>->lock);
>> +write_lock(>->lock);
>>  
>>  /* Bounds check on the grant refs */
>>  if ( unlikely(ref_a >= nr_grant_entries(d->grant_table)))
>> @@ -2689,7 +2707,7 @@ out:
>>  active_e

Re: [Xen-devel] [PATCHv10 4/4] gnttab: use per-VCPU maptrack free lists

[David Vrabel]

On 28/05/15 16:39, Jan Beulich wrote:
 On 26.05.15 at 20:00,  wrote:
>> --- a/xen/common/grant_table.c
>> +++ b/xen/common/grant_table.c
>> @@ -57,7 +57,7 @@ integer_param("gnttab_max_frames", max_grant_frames);
>>   * New options allow to set max_maptrack_frames and
>>   * map_grant_table_frames independently.
>>   */
>> -#define DEFAULT_MAX_MAPTRACK_FRAMES 256
>> +#define DEFAULT_MAX_MAPTRACK_FRAMES 1024
> 
> Apart from everything else this again results in ...
> 
>> @@ -1457,6 +1491,17 @@ gnttab_setup_table(
>>  gt = d->grant_table;
>>  write_lock(>->lock);
>>  
>> +/* Tracking of mapped foreign frames table */
>> +gt->maptrack = xzalloc_array(struct grant_mapping *, 
>> max_maptrack_frames);
> 
> ... this becoming an order-1 runtime allocation on other than 32-bit
> ARM.

I thought we agreed that this was temporary until vzalloc() was added?

David

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

Re: [Xen-devel] [PATCH V7] xen/vm_event: Clean up control-register-write vm_events and add XCR0 event

[Lengyel, Tamas]

On Thu, May 28, 2015 at 10:33 AM, Razvan Cojocaru  wrote:

> On 05/28/2015 05:12 PM, Lengyel, Tamas wrote:
> > diff --git a/xen/arch/x86/hvm/event.c b/xen/arch/x86/hvm/event.c
> > index 9d5f9f3..a13195d 100644
> > --- a/xen/arch/x86/hvm/event.c
> > +++ b/xen/arch/x86/hvm/event.c
> > @@ -21,6 +21,7 @@
> >
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >
> >  static void hvm_event_fill_regs(vm_event_request_t *req)
> > @@ -88,55 +89,28 @@ static int hvm_event_traps(uint8_t sync,
> > vm_event_request_t *req)
> >  return 1;
> >  }
> >
> > -static inline
> > -void hvm_event_cr(uint32_t reason, unsigned long value,
> > - unsigned long old, bool_t onchangeonly,
> > bool_t sync)
> > +void (hvm_event_cr)(unsigned int index, unsigned long value,
> > unsigned long old)
> >
> >
> > Any reason to have parenthesis around hvm_event_cr? Also, if you could
> > make the patch available somewhere via git, that would be great, I would
> > like to test it!
>
> Yes, to prevent macro expansion. Jan has suggested that I add a macro to
> make the calls look neater, please see this change:
>
> +void hvm_event_cr(unsigned int index, unsigned long value, unsigned
> long old);
> +#define hvm_event_cr(what, new, old) hvm_event_cr(VM_EVENT_X86_##what,
> new, old)
>
> So, unless that call is put in parenthesis, the code you mention is
> expanded to hvm_event_cr(VM_EVENT_X86_unsigned int index, ...) or
> something of that nature, and it won't compile.
>

That seems a bit odd to me that we now have both a macro and a function
with the same name. I think making the macro at least all capital would
avoid confusion and make the code a lot more readable. But even then IMHO
this wrapper is not helping code readability much. Its really a pain to
track down auto-expanding fields in macros when someone is trying to read
the code.


>
> As for a git repo, unfortunately I don't have a public one, but I intend
> to submit V8 shortly, and if I understood things correctly it's quite
> likely to enter staging, so it should only be a short while now.
>

I guess but putting it up on github as well should really not be a problem..


>
> Thanks,
> Razvan
>



-- 

[image: www.novetta.com]

Tamas K Lengyel

Senior Security Researcher

7921 Jones Branch Drive

McLean VA 22102

Email  tleng...@novetta.com
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [libvirt] [PATCH 3/4] libxl: support SPICE graphics for HVM domains

[Michal Privoznik]

On 28.05.2015 17:45, Jim Fehlig wrote:
> 
> 
>> On May 28, 2015, at 4:07 AM, Michal Privoznik  wrote:
>>
>>> On 09.05.2015 00:31, Jim Fehlig wrote:
>>> Signed-off-by: Jim Fehlig 
>>> ---
>>> src/libxl/libxl_conf.c | 72 
>>> +-
>>> 1 file changed, 66 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
>>> index 8552c77..5bb0425 100644
>>> --- a/src/libxl/libxl_conf.c
>>> +++ b/src/libxl/libxl_conf.c
>>> @@ -1337,22 +1337,82 @@ libxlMakeVfbList(virPortAllocatorPtr graphicsports,
>>>
>>> /*
>>>  * Populate vfb info in libxl_domain_build_info struct for HVM domains.
>>> - * Use first libxl_device_vfb device in libxl_domain_config->vfbs.
>>> - * Prior to calling this function, libxlMakeVfbList must be called to
>>> - * populate libxl_domain_config->vfbs.
>>> + * Prefer SPICE, otherwise use first libxl_device_vfb device in
>>> + * libxl_domain_config->vfbs. Prior to calling this function,
>>> + * libxlMakeVfbList must be called to populate libxl_domain_config->vfbs.
>>>  */
>>> static int
>>> -libxlMakeBuildInfoVfb(virDomainDefPtr def, libxl_domain_config *d_config)
>>> +libxlMakeBuildInfoVfb(virPortAllocatorPtr graphicsports,
>>> +  virDomainDefPtr def,
>>> +  libxl_domain_config *d_config)
>>> {
>>> libxl_domain_build_info *b_info = &d_config->b_info;
>>> libxl_device_vfb x_vfb;
>>> +size_t i;
>>>
>>> if (def->os.type != VIR_DOMAIN_OSTYPE_HVM)
>>> return 0;
>>>
>>> -if (d_config->num_vfbs == 0)
>>> +if (def->ngraphics == 0)
>>> return 0;
>>>
>>> +for (i = 0; i < def->ngraphics; i++) {
>>> +virDomainGraphicsDefPtr l_vfb = def->graphics[0];
>>
>> This seems really awkward to me. So you're using for() loop just to
>> check if the first graphics card (assuming there can't be more than one
>> anyway) is SPICE. If not, you could use 'continue' to continue with VNC.
>> I think this obfucates the code. Just move this into a separate function
>> and call it from here.
> 
> That's actually a bug, it should be def->graphics[i].  The idea is to prefer 
> SPICE, but fall back to the first graphics device if no SPICE device is 
> found. I mentioned this in the function comment. Perhaps that part of the 
> comment should be moved to the for loop?
> 

Yes, that sounds reasonable to me.

Michal

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

Re: [Xen-devel] [libvirt] [PATCH 3/4] libxl: support SPICE graphics for HVM domains

[Jim Fehlig]

> On May 28, 2015, at 4:07 AM, Michal Privoznik  wrote:
> 
>> On 09.05.2015 00:31, Jim Fehlig wrote:
>> Signed-off-by: Jim Fehlig 
>> ---
>> src/libxl/libxl_conf.c | 72 
>> +-
>> 1 file changed, 66 insertions(+), 6 deletions(-)
>> 
>> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
>> index 8552c77..5bb0425 100644
>> --- a/src/libxl/libxl_conf.c
>> +++ b/src/libxl/libxl_conf.c
>> @@ -1337,22 +1337,82 @@ libxlMakeVfbList(virPortAllocatorPtr graphicsports,
>> 
>> /*
>>  * Populate vfb info in libxl_domain_build_info struct for HVM domains.
>> - * Use first libxl_device_vfb device in libxl_domain_config->vfbs.
>> - * Prior to calling this function, libxlMakeVfbList must be called to
>> - * populate libxl_domain_config->vfbs.
>> + * Prefer SPICE, otherwise use first libxl_device_vfb device in
>> + * libxl_domain_config->vfbs. Prior to calling this function,
>> + * libxlMakeVfbList must be called to populate libxl_domain_config->vfbs.
>>  */
>> static int
>> -libxlMakeBuildInfoVfb(virDomainDefPtr def, libxl_domain_config *d_config)
>> +libxlMakeBuildInfoVfb(virPortAllocatorPtr graphicsports,
>> +  virDomainDefPtr def,
>> +  libxl_domain_config *d_config)
>> {
>> libxl_domain_build_info *b_info = &d_config->b_info;
>> libxl_device_vfb x_vfb;
>> +size_t i;
>> 
>> if (def->os.type != VIR_DOMAIN_OSTYPE_HVM)
>> return 0;
>> 
>> -if (d_config->num_vfbs == 0)
>> +if (def->ngraphics == 0)
>> return 0;
>> 
>> +for (i = 0; i < def->ngraphics; i++) {
>> +virDomainGraphicsDefPtr l_vfb = def->graphics[0];
> 
> This seems really awkward to me. So you're using for() loop just to
> check if the first graphics card (assuming there can't be more than one
> anyway) is SPICE. If not, you could use 'continue' to continue with VNC.
> I think this obfucates the code. Just move this into a separate function
> and call it from here.

That's actually a bug, it should be def->graphics[i].  The idea is to prefer 
SPICE, but fall back to the first graphics device if no SPICE device is found. 
I mentioned this in the function comment. Perhaps that part of the comment 
should be moved to the for loop?

Regards,
Jim

> 
>> +unsigned short port;
>> +const char *listenAddr;
>> +
>> +if (l_vfb->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE)
>> +continue;
>> +
>> +libxl_defbool_set(&b_info->u.hvm.spice.enable, true);
>> +
>> +if (l_vfb->data.spice.autoport) {
>> +if (virPortAllocatorAcquire(graphicsports, &port) < 0)
>> +return -1;
>> +l_vfb->data.spice.port = port;
>> +}
>> +b_info->u.hvm.spice.port = l_vfb->data.spice.port;
>> +
>> +listenAddr = virDomainGraphicsListenGetAddress(l_vfb, 0);
>> +if (VIR_STRDUP(b_info->u.hvm.spice.host, listenAddr) < 0)
>> +return -1;
>> +
>> +if (VIR_STRDUP(b_info->u.hvm.keymap, l_vfb->data.spice.keymap) < 0)
>> +return -1;
>> +
>> +if (l_vfb->data.spice.auth.passwd) {
>> +if (VIR_STRDUP(b_info->u.hvm.spice.passwd,
>> +   l_vfb->data.spice.auth.passwd) < 0)
>> +return -1;
>> +libxl_defbool_set(&b_info->u.hvm.spice.disable_ticketing, 
>> false);
>> +} else {
>> +libxl_defbool_set(&b_info->u.hvm.spice.disable_ticketing, true);
>> +}
>> +
>> +switch (l_vfb->data.spice.mousemode) {
>> +/* client mouse mode is default in xl.cfg */
>> +case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_DEFAULT:
>> +case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_CLIENT:
>> +libxl_defbool_set(&b_info->u.hvm.spice.agent_mouse, true);
>> +break;
>> +case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_SERVER:
>> +libxl_defbool_set(&b_info->u.hvm.spice.agent_mouse, false);
>> +break;
>> +}
>> +
>> +#ifdef LIBXL_HAVE_SPICE_VDAGENT
>> +if (l_vfb->data.spice.copypaste == VIR_TRISTATE_BOOL_YES) {
>> +libxl_defbool_set(&b_info->u.hvm.spice.vdagent, true);
>> +libxl_defbool_set(&b_info->u.hvm.spice.clipboard_sharing, true);
>> +} else {
>> +libxl_defbool_set(&b_info->u.hvm.spice.vdagent, false);
>> +libxl_defbool_set(&b_info->u.hvm.spice.clipboard_sharing, 
>> false);
>> +}
>> +#endif
>> +
>> +return 0;
>> +}
>> +
>> x_vfb = d_config->vfbs[0];
>> 
>> if (libxl_defbool_val(x_vfb.vnc.enable)) {
>> @@ -1778,7 +1838,7 @@ libxlBuildDomainConfig(virPortAllocatorPtr 
>> graphicsports,
>> if (libxlMakeVfbList(graphicsports, def, d_config) < 0)
>> return -1;
>> 
>> -if (libxlMakeBuildInfoVfb(def, d_config) < 0)
>> +if (libxlMakeBuildInfoVfb(graphicsports, def, d_config) < 0)
>> return -1;
>> 
>> if (libxlMakePCIList(def, d_config) < 0)
> 
> Otherwise looking good.

Re: [Xen-devel] [PATCH] dmar: device scope mem leak fix

[Elena Ufimtseva]

On Thu, May 28, 2015 at 08:57:16AM +0100, Jan Beulich wrote:
> >>> On 27.05.15 at 21:56,  wrote:
> > On Tue, May 26, 2015 at 10:46:30AM +0100, Jan Beulich wrote:
> >> >>> On 23.05.15 at 03:27,  wrote:
> >> > @@ -658,6 +661,7 @@ acpi_parse_one_rmrr(struct acpi_dmar_header *header)
> >> >  "  Ignore the RMRR (%"PRIx64", %"PRIx64") due to "
> >> >  "devices under its scope are not PCI discoverable!\n",
> >> >  rmrru->base_address, rmrru->end_address);
> >> > +xfree(rmrru->scope.devices);
> >> >  xfree(rmrru);
> > Do you think the ret should be set in this case also?
> 
> Iirc in an earlier version of the other series you had added a failure
> error code setting here, and I had to specifically ask you to remove
> it. If you still think one is needed here, this would need to be a
> separate patch with a proper explanation.
> 
> Jan

Hi Jan

I do remember this. Looked again and I think it makes sense on ignore
path not to return error.


> 

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

Re: [Xen-devel] [PATCHv10 4/4] gnttab: use per-VCPU maptrack free lists

[Jan Beulich]

>>> On 26.05.15 at 20:00,  wrote:
> v10:
> - Divide max_maptrack_frames evenly amongst the VCPUs.
> - Increase default max_maptrack_frames to compensate.

I'm not really happy about this, but also can't immediately suggest
a better model (without removing maptrack frames from vCPU-s). I'd
really like to hear opinions of others, namely maintainers, regarding
this resource use issue. Should we perhaps start accounting
maptrack frames against the domain's allocation (i.e. a domain can
get as many as it likes beyond the set limit, provided it ballooned
out suitably many pages up front)?

> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -57,7 +57,7 @@ integer_param("gnttab_max_frames", max_grant_frames);
>   * New options allow to set max_maptrack_frames and
>   * map_grant_table_frames independently.
>   */
> -#define DEFAULT_MAX_MAPTRACK_FRAMES 256
> +#define DEFAULT_MAX_MAPTRACK_FRAMES 1024

Apart from everything else this again results in ...

> @@ -1457,6 +1491,17 @@ gnttab_setup_table(
>  gt = d->grant_table;
>  write_lock(>->lock);
>  
> +/* Tracking of mapped foreign frames table */
> +gt->maptrack = xzalloc_array(struct grant_mapping *, 
> max_maptrack_frames);

... this becoming an order-1 runtime allocation on other than 32-bit
ARM.

Jan


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

Re: [Xen-devel] [PATCH v22 11/14] x86/VPMU: Handle PMU interrupts for PV(H) guests

[Boris Ostrovsky]

On 05/28/2015 11:05 AM, Aravind Gopalakrishnan wrote:

On 5/26/2015 1:09 PM, Boris Ostrovsky wrote:

On 05/26/2015 12:24 PM, Jan Beulich wrote:

On 21.05.15 at 19:57,  wrote:

@@ -188,27 +189,52 @@ static inline void context_load(struct vcpu *v)
  }
  }
  -static void amd_vpmu_load(struct vcpu *v)
+static int amd_vpmu_load(struct vcpu *v, bool_t from_guest)
  {
  struct vpmu_struct *vpmu = vcpu_vpmu(v);
-struct xen_pmu_amd_ctxt *ctxt = vpmu->context;
-uint64_t *ctrl_regs = vpmu_reg_pointer(ctxt, ctrls);
+struct xen_pmu_amd_ctxt *ctxt;
+uint64_t *ctrl_regs;
+unsigned int i;
vpmu_reset(vpmu, VPMU_FROZEN);
  -if ( vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
+if ( !from_guest && vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
  {
-unsigned int i;
+ctxt = vpmu->context;
+ctrl_regs = vpmu_reg_pointer(ctxt, ctrls);
for ( i = 0; i < num_counters; i++ )
  wrmsrl(ctrls[i], ctrl_regs[i]);
  -return;
+return 0;
+}
+
+if ( from_guest )
+{
+ASSERT(!is_hvm_vcpu(v));
+
+ctxt = &vpmu->xenpmu_data->pmu.c.amd;
+ctrl_regs = vpmu_reg_pointer(ctxt, ctrls);
+for ( i = 0; i < num_counters; i++ )
+{
+if ( is_pmu_enabled(ctrl_regs[i]) )
+{
+vpmu_set(vpmu, VPMU_RUNNING);
+break;
+}
+}
+
+if ( i == num_counters )
+vpmu_reset(vpmu, VPMU_RUNNING);
+
+memcpy(vpmu->context, &vpmu->xenpmu_data->pmu.c.amd, 
ctxt_sz);

  }
vpmu_set(vpmu, VPMU_CONTEXT_LOADED);
context_load(v);
+
+return 0;
  }

So no verification needed at all on the AMD side? If so,



So I went back to BKDGs and it looks like some models of family 15 
redefined one of the bits from Reserved to MBZ so I think I'll need 
to verify that bit now.


It's rather strange that this bit (MSRC001_0200[19]) is reserved for 
models 00h-0Fh and 30-3Fh but is MBZ for 10h-1Fh. It is also reserved 
for families 10h and 16h. I don't have access to the MBZ models so I 
can't test whether it is indeed MBZ or a typo in the spec (I can 
certainly write it with 1 on family 10h and 15h/model2).



So I asked about it internally and it seems it is indeed a BKDG error. 
The bit is 'Reserved'.

I also tried writing 1 to it on Fam15h Model10h and it works fine.


Excellent, thanks Aravind!

As Jan pointed out though for Reserved fields we still may need to 
preserve bits if we are to follow BKDG to the letter (although writing 
them doesn't seem to have any effect, at least on processors that I tried).


And in fact we don't check those bits currently neither so I think I'll 
add a separate patch to verify them in amd_vpmu_do_wrmsr().


-boris


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

Re: [Xen-devel] [PATCH v22 11/14] x86/VPMU: Handle PMU interrupts for PV(H) guests

[Aravind Gopalakrishnan]

On 5/26/2015 1:09 PM, Boris Ostrovsky wrote:

On 05/26/2015 12:24 PM, Jan Beulich wrote:

On 21.05.15 at 19:57,  wrote:

@@ -188,27 +189,52 @@ static inline void context_load(struct vcpu *v)
  }
  }
  -static void amd_vpmu_load(struct vcpu *v)
+static int amd_vpmu_load(struct vcpu *v, bool_t from_guest)
  {
  struct vpmu_struct *vpmu = vcpu_vpmu(v);
-struct xen_pmu_amd_ctxt *ctxt = vpmu->context;
-uint64_t *ctrl_regs = vpmu_reg_pointer(ctxt, ctrls);
+struct xen_pmu_amd_ctxt *ctxt;
+uint64_t *ctrl_regs;
+unsigned int i;
vpmu_reset(vpmu, VPMU_FROZEN);
  -if ( vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
+if ( !from_guest && vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
  {
-unsigned int i;
+ctxt = vpmu->context;
+ctrl_regs = vpmu_reg_pointer(ctxt, ctrls);
for ( i = 0; i < num_counters; i++ )
  wrmsrl(ctrls[i], ctrl_regs[i]);
  -return;
+return 0;
+}
+
+if ( from_guest )
+{
+ASSERT(!is_hvm_vcpu(v));
+
+ctxt = &vpmu->xenpmu_data->pmu.c.amd;
+ctrl_regs = vpmu_reg_pointer(ctxt, ctrls);
+for ( i = 0; i < num_counters; i++ )
+{
+if ( is_pmu_enabled(ctrl_regs[i]) )
+{
+vpmu_set(vpmu, VPMU_RUNNING);
+break;
+}
+}
+
+if ( i == num_counters )
+vpmu_reset(vpmu, VPMU_RUNNING);
+
+memcpy(vpmu->context, &vpmu->xenpmu_data->pmu.c.amd, ctxt_sz);
  }
vpmu_set(vpmu, VPMU_CONTEXT_LOADED);
context_load(v);
+
+return 0;
  }

So no verification needed at all on the AMD side? If so,



So I went back to BKDGs and it looks like some models of family 15 
redefined one of the bits from Reserved to MBZ so I think I'll need to 
verify that bit now.


It's rather strange that this bit (MSRC001_0200[19]) is reserved for 
models 00h-0Fh and 30-3Fh but is MBZ for 10h-1Fh. It is also reserved 
for families 10h and 16h. I don't have access to the MBZ models so I 
can't test whether it is indeed MBZ or a typo in the spec (I can 
certainly write it with 1 on family 10h and 15h/model2).



So I asked about it internally and it seems it is indeed a BKDG error. 
The bit is 'Reserved'.

I also tried writing 1 to it on Fam15h Model10h and it works fine.

Thanks,
-Aravind.

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

Re: [Xen-devel] [PATCH v7 04/10] xen: Introduce XEN_DOMCTL_soft_reset

[Jan Beulich]

>>> On 28.05.15 at 15:53,  wrote:
> Tim Deegan  writes:
>> BTW having looked at how messy this is ending up, and how it's still
>> incomplete, I'd agree with Jan that resetting the domain state might
>> be a better approach.
> 
> Even with the 'reset-everything' approach the function from this patch
> will still be required in some form as we'll still have to walk the p2m
> and each individual page checking it's count_info and replacing in case
> of need. At the same time we'll have lots of other hypervisor-related
> implications (tearing down everything) so I seriously doubt it's going
> to end up less messy (toolstack-related changes might go away though).

That would remain to be seen. I'm particularly unconvinced that
iterating over all pages an copying is going to be needed: Iirc it
was only grants mapped by another domain that we really need
to hunt down, and we should be able to do that without iterating
over the whole GFN space of the domain (or all of its assigned
pages).

Jan


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

Re: [Xen-devel] [PATCHv10 3/4] gnttab: make the grant table lock a read-write lock

[Jan Beulich]

>>> On 26.05.15 at 20:00,  wrote:
> @@ -254,23 +254,23 @@ double_gt_lock(struct grant_table *lgt, struct 
> grant_table *rgt)
>  {
>  if ( lgt < rgt )
>  {
> -spin_lock(&lgt->lock);
> -spin_lock(&rgt->lock);
> +write_lock(&lgt->lock);
> +write_lock(&rgt->lock);
>  }
>  else
>  {
>  if ( lgt != rgt )
> -spin_lock(&rgt->lock);
> -spin_lock(&lgt->lock);
> +write_lock(&rgt->lock);
> +write_lock(&lgt->lock);
>  }
>  }

So I looked at the two uses of double_gt_lock() again: in both cases
only a read lock is needed on rgt (which is also the natural thing to
expect: we aren't supposed to modify the remote domain's grant
table in any way here). Albeit that's contradicting ...

> @@ -568,10 +568,10 @@ static void mapcount(
>  *wrc = *rdc = 0;
>  
>  /*
> - * Must have the remote domain's grant table lock while counting
> - * its active entries.
> + * Must have the remote domain's grant table write lock while
> + * counting its active entries.
>   */
> -ASSERT(spin_is_locked(&rd->grant_table->lock));
> +ASSERT(rw_is_write_locked(&rd->grant_table->lock));

... this: Why would we need to hold the write lock here? We're
not changing anything in rd->grant_table.

> @@ -837,12 +838,22 @@ __gnttab_map_grant_ref(
>  
>  TRACE_1D(TRC_MEM_PAGE_GRANT_MAP, op->dom);
>  
> +/*
> + * All maptrack entry users check mt->flags first before using the
> + * other fields so just ensure the flags field is stored last.
> + *
> + * However, if gnttab_need_iommu_mapping() then this would race
> + * with a concurrent mapcount() call (on an unmap, for example)
> + * and a lock is required.
> + */
>  mt = &maptrack_entry(lgt, handle);
>  mt->domid = op->dom;
>  mt->ref   = op->ref;
> -mt->flags = op->flags;
> +wmb();
> +write_atomic(&mt->flags, op->flags);

Doesn't that then also require the use of read_atomic() and rmb()
on the reading side? Further, why are only races against mapcount()
a problem, but not such against __gnttab_unmap_common() as a
whole? I.e. what's the locking around the op->map->flags and
op->map->domid accesses below good for? Or, alternatively, isn't
this an indication of a problem with the previous patch splitting off
the maptrack lock (effectively leaving some map track entry
accesses without any guard)?

> -double_gt_unlock(lgt, rgt);
> +if ( gnttab_need_iommu_mapping(ld) )
> +double_gt_unlock(lgt, rgt);

I think you shouldn't rely on gnttab_need_iommu_mapping() to
produce the same result upon this and the earlier invocation, i.e.
you ought to latch the first call's result into a local variable.

> @@ -1787,7 +1805,7 @@ gnttab_transfer(
>  TRACE_1D(TRC_MEM_PAGE_GRANT_TRANSFER, e->domain_id);
>  
>  /* Tell the guest about its new page frame. */
> -spin_lock(&e->grant_table->lock);
> +read_lock(&e->grant_table->lock);
>  
>  if ( e->grant_table->gt_version == 1 )
>  {
> @@ -1805,7 +1823,7 @@ gnttab_transfer(
>  shared_entry_header(e->grant_table, gop.ref)->flags |=
>  GTF_transfer_completed;
>  
> -spin_unlock(&e->grant_table->lock);
> +read_unlock(&e->grant_table->lock);

I'm not sure a read lock suffices here: Consider the effect of two
parallel invocations of this code with identical gop.ref, but different
gop.mfn. Or wait, gnttab_prepare_for_transfer() makes sure only
one gets here, unless the granting domain fiddles with the shared
entry. But it feels wrong nevertheless, considering that we alter
state of e here. Or should this perhaps lock the matching active
entry, even if it doesn't touch it?

> @@ -2645,7 +2663,7 @@ __gnttab_swap_grant_ref(grant_ref_t ref_a, grant_ref_t 
> ref_b)
>  struct active_grant_entry *act_b = NULL;
>  s16 rc = GNTST_okay;
>  
> -spin_lock(>->lock);
> +write_lock(>->lock);
>  
>  /* Bounds check on the grant refs */
>  if ( unlikely(ref_a >= nr_grant_entries(d->grant_table)))
> @@ -2689,7 +2707,7 @@ out:
>  active_entry_release(act_b);
>  if ( act_a != NULL )
>  active_entry_release(act_a);
> -spin_unlock(>->lock);
> +write_unlock(>->lock);

It would seem to me that these could be dropped when the per-active-
entry locks get introduced.

Jan

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

Re: [Xen-devel] [PATCH V7] xen/vm_event: Clean up control-register-write vm_events and add XCR0 event

[Razvan Cojocaru]

On 05/28/2015 05:12 PM, Lengyel, Tamas wrote:
> diff --git a/xen/arch/x86/hvm/event.c b/xen/arch/x86/hvm/event.c
> index 9d5f9f3..a13195d 100644
> --- a/xen/arch/x86/hvm/event.c
> +++ b/xen/arch/x86/hvm/event.c
> @@ -21,6 +21,7 @@
> 
>  #include 
>  #include 
> +#include 
>  #include 
> 
>  static void hvm_event_fill_regs(vm_event_request_t *req)
> @@ -88,55 +89,28 @@ static int hvm_event_traps(uint8_t sync,
> vm_event_request_t *req)
>  return 1;
>  }
> 
> -static inline
> -void hvm_event_cr(uint32_t reason, unsigned long value,
> - unsigned long old, bool_t onchangeonly,
> bool_t sync)
> +void (hvm_event_cr)(unsigned int index, unsigned long value,
> unsigned long old)
> 
> 
> Any reason to have parenthesis around hvm_event_cr? Also, if you could
> make the patch available somewhere via git, that would be great, I would
> like to test it!

Yes, to prevent macro expansion. Jan has suggested that I add a macro to
make the calls look neater, please see this change:

+void hvm_event_cr(unsigned int index, unsigned long value, unsigned
long old);
+#define hvm_event_cr(what, new, old) hvm_event_cr(VM_EVENT_X86_##what,
new, old)

So, unless that call is put in parenthesis, the code you mention is
expanded to hvm_event_cr(VM_EVENT_X86_unsigned int index, ...) or
something of that nature, and it won't compile.

As for a git repo, unfortunately I don't have a public one, but I intend
to submit V8 shortly, and if I understood things correctly it's quite
likely to enter staging, so it should only be a short while now.


Thanks,
Razvan

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

[Xen-devel] [xen-4.4-testing test] 57411: regressions - FAIL

[osstest service user]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemuu-win7-amd64 14 guest-localmigrate.2 fail REGR. vs. 
56796

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-multivcpu 11 guest-start  fail  like 56796
 test-amd64-i386-xend-qemut-winxpsp3 16 guest-stop  fail like 56796

Tests which did not succeed, but are not blocking:
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)   blocked n/a
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2   6 xen-boot fail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 11 guest-start  fail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-sedf-pin 12 migrate-support-checkfail   never pass
 build-amd64-rumpuserxen   6 xen-buildfail   never pass
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail never pass
 build-i386-rumpuserxen6 xen-buildfail   never pass
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-sedf 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass

version targeted for testing:
 xen  5624637dc81624dc13f8db559def61538cccbafe
baseline version:
 xen  226059859e6f5a80a5a1728e5dc8ccde50e25f80


People who touched revisions under test:
  Ian Jackson 
  Petr Matousek 


jobs:
 build-amd64-xend pass
 build-i386-xend  pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 build-amd64-rumpuserxen  fail
 build-i386-rumpuserxen   fail
 test-amd64-amd64-xl  pass
 test-armhf-armhf-xl  pass
 test-amd64-i386-xl   pass
 test-amd64-i386-qemut-rhel6hvm-amd   pass
 test-amd64-i386-qemuu-rhel6hvm-amd   pass
 test-amd64-amd64-xl-qemut-debianhvm-amd64pass
 test-amd64-i386-xl-qemut-debianhvm-amd64 pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-i386-xl-qemuu-debianhvm-amd64 pass
 test-amd64-i386-freebsd10-amd64  pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  pass
 test-amd64-amd64-rumpuserxen-amd64   blocked 
 test-amd64-amd64-xl-qemut-win7-amd64 fail
 test-amd64-i386-xl-qemut-win7-amd64  fail
 test-amd64-amd64-xl-qemuu-win7-amd64 pass
 test-amd64-i386-xl-qemuu-win7-amd64  fail
 test-armhf-armhf-xl-arndale  pass
 test-amd64-amd64-xl-credit2  pass
 test-armhf-armhf-xl-credit2  fail
 test-armhf-armhf-xl-cubietruck   pass
 test-amd64-i386-freebsd10-i386   pass
 test-amd64-i386-rumpuserxen-i386 blocked 
 test-amd64-i386-qemut-rhel6hvm-intel pass
 test-amd64-i386-qemuu-rhel6hvm-intel pass
 test-amd64-amd64-libvirt pass
 test-armhf-armhf-libvirt fail
 test-amd64-i386-libvirt 

Re: [Xen-devel] [PATCH V7] xen/vm_event: Clean up control-register-write vm_events and add XCR0 event

[Lengyel, Tamas]

diff --git a/xen/arch/x86/hvm/event.c b/xen/arch/x86/hvm/event.c
> index 9d5f9f3..a13195d 100644
> --- a/xen/arch/x86/hvm/event.c
> +++ b/xen/arch/x86/hvm/event.c
> @@ -21,6 +21,7 @@
>
>  #include 
>  #include 
> +#include 
>  #include 
>
>  static void hvm_event_fill_regs(vm_event_request_t *req)
> @@ -88,55 +89,28 @@ static int hvm_event_traps(uint8_t sync,
> vm_event_request_t *req)
>  return 1;
>  }
>
> -static inline
> -void hvm_event_cr(uint32_t reason, unsigned long value,
> - unsigned long old, bool_t onchangeonly, bool_t
> sync)
> +void (hvm_event_cr)(unsigned int index, unsigned long value, unsigned
> long old)
>

Any reason to have parenthesis around hvm_event_cr? Also, if you could make
the patch available somewhere via git, that would be great, I would like to
test it!

Thanks!


-- 

[image: www.novetta.com]

Tamas K Lengyel

Senior Security Researcher

7921 Jones Branch Drive

McLean VA 22102

Email  tleng...@novetta.com
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v7 04/10] xen: Introduce XEN_DOMCTL_soft_reset

[Vitaly Kuznetsov]

Tim Deegan  writes:

> At 15:11 +0200 on 28 May (1432825919), Vitaly Kuznetsov wrote:
>> Tim Deegan  writes:
>> > At 13:56 +0200 on 28 May (1432821360), Vitaly Kuznetsov wrote:
>> >> Tim Deegan  writes:
>> >> >> +while ( next_page && !is_xen_heap_page(next_page) &&
>> >> >> +page_to_mfn(next_page) == mfn + count )
>> >> >
>> >> > What's the purpose of this second loop?  It doesn't seem to be doing
>> >> > anything that the outer loop couldn't.
>> >> 
>> >> True. This second loops searches for a continuous region to preserve the
>> >> order of mappings (when possible)
>> >
>> > Ah; I think this, like the PoD case, should the more detailed p2m
>> > lookup to get the actual order of the current mapping.  That should be
>> > shorter and more readable, and probably more correct.
>> 
>> If we bring get_gfn_type_access() call to the top level it becomes
>> possible (and easy) but (if I'm not mistaken) we still need to walk
>> through all pages of the mapping checking that each of them has the
>> reqiuired count_info (so it is not mapped from other domain or xen
>> itself). In case we meet a 'bad' page we'll have to split the mapping
>> (and copy the page itself).
>
> Hmmm.  Yes, we can only reassign one page at a time.  I think that
> will look cleaner if you split out the reassign-or-copy into a
> separate function that takes a start + order and DTRT, and then having
> the loop in this function handle one p2m entry (of whatever order) per
> iteration.
>
> BTW having looked at how messy this is ending up, and how it's still
> incomplete, I'd agree with Jan that resetting the domain state might
> be a better approach.

Even with the 'reset-everything' approach the function from this patch
will still be required in some form as we'll still have to walk the p2m
and each individual page checking it's count_info and replacing in case
of need. At the same time we'll have lots of other hypervisor-related
implications (tearing down everything) so I seriously doubt it's going
to end up less messy (toolstack-related changes might go away though).

-- 
  Vitaly

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

Re: [Xen-devel] [PATCHv10 2/4] gnttab: introduce maptrack lock

[Jan Beulich]

>>> On 26.05.15 at 20:00,  wrote:
> Split grant table lock into two separate locks. One to protect
> maptrack state (maptrack_lock) and one for everything else (lock).
> 
> Based on a patch originally by Matt Wilson .
> 
> Signed-off-by: David Vrabel 

Reviewed-by: Jan Beulich 


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

Re: [Xen-devel] [PATCHv10 1/4] gnttab: per-active entry locking

[Jan Beulich]

>>> On 26.05.15 at 20:00,  wrote:
> @@ -514,18 +534,19 @@ static int grant_map_exists(const struct domain *ld,
> nr_grant_entries(rgt));
>  for ( ref = *ref_count; ref < max_iter; ref++ )
>  {
> -act = &active_entry(rgt, ref);
> +struct active_grant_entry *act;
> +bool_t exists;
>  
> -if ( !act->pin )
> -continue;
> +act = active_entry_acquire(rgt, ref);
>  
> -if ( act->domid != ld->domain_id )
> -continue;
> +exists = act->pin
> +&& act->domid == ld->domain_id
> +&& act->frame != mfn;

== (you're inverting the entire but split up original || expression,
i.e. namely ...

> -if ( act->frame != mfn )
> -continue;

... also this one)

With that fixed,
Reviewed-by: Jan Beulich 


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

Re: [Xen-devel] [PATCH v7 00/10] toolstack-based approach to pvhvm guest kexec

[Vitaly Kuznetsov]

"Jan Beulich"  writes:

 On 28.05.15 at 14:27,  wrote:
>> "Jan Beulich"  writes:
>> 
>> On 27.05.15 at 17:25,  wrote:
 This patch series provides x86 PVHVM domains with an ability to perform
 kexec/kdump-style operations.
>>>
>>> Before I get to look at this latest version, may I go a step back and
>>> ask for clarification whether all of these (seemingly fragile)
>>> manipulations are actually needed (such a rationale for the series
>>> would btw be quite good to have in the overview mail, rather than
>>> having to wade through mailing list history). In particular, why is it
>>> that we need a new domain in the first place? After all, native
>>> kexec doesn't imply a new physical machine either. Perhaps that
>>> was discussed a long while back, but I can't seem to find any of
>>> that now that I would like to read through it again.
>> 
>> Yes, here are some highlights from last year's discussion:
>> 
>> http://lists.xen.org/archives/html/xen-devel/2014-08/msg01908.html 
>> 
>> http://lists.xen.org/archives/html/xen-devel/2014-08/msg01979.html 
>
> I.e. what you currently implement is David's model without Konrad's
> later alternative really having been explored? Iiuc David's main
> reservation (which I share) was against a myriad of reset-this and
> reset-that hypercalls, which Konrad's reset-everything would
> address equally well.

I was under an impression Konrad was also suggesting building a new
domain with "instead of chasing down different states (event channels,
grants, vcpu's, pagetables, etc) we would wipe everything to a nice
clean slate" (as we'll have to chase down all this bits with a
'reset-everything' hypercall) but now I'm not sure.

AFAICT If we follow down the 'reset-everything' road it's not going to
be any easier and less fragile. E.g. I don't see an easy way to deal
with grants: even if we can clean all the internal bits we still have
pages mapped to the backend domain (Qemu and other backends) and there
is no easy way to ask them to unmap everything. We could (in theory)
walk through all the pages of our domain replacing all pages which need
replacement with empty pages but we need to temporary assign old pages
somewhere, avoid over-allocation (as e.g. in tot_pages = max_pages case
we can't add a single page to the domain). The support burden of a
'reset-everything' hypercall is also going to be heavier as all newly
added bits will have to be added to it.

The approach used in this series is not significantly different from how
an HVM domain is doing normal reboot: we destroy the original domain and
create a new one instead of cleaning up the original one (as it looks
safer and much easier I suppose).

-- 
  Vitaly

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

Re: [Xen-devel] [PATCH v7 04/10] xen: Introduce XEN_DOMCTL_soft_reset

[Tim Deegan]

At 15:11 +0200 on 28 May (1432825919), Vitaly Kuznetsov wrote:
> Tim Deegan  writes:
> > At 13:56 +0200 on 28 May (1432821360), Vitaly Kuznetsov wrote:
> >> Tim Deegan  writes:
> >> >> +while ( next_page && !is_xen_heap_page(next_page) &&
> >> >> +page_to_mfn(next_page) == mfn + count )
> >> >
> >> > What's the purpose of this second loop?  It doesn't seem to be doing
> >> > anything that the outer loop couldn't.
> >> 
> >> True. This second loops searches for a continuous region to preserve the
> >> order of mappings (when possible)
> >
> > Ah; I think this, like the PoD case, should the more detailed p2m
> > lookup to get the actual order of the current mapping.  That should be
> > shorter and more readable, and probably more correct.
> 
> If we bring get_gfn_type_access() call to the top level it becomes
> possible (and easy) but (if I'm not mistaken) we still need to walk
> through all pages of the mapping checking that each of them has the
> reqiuired count_info (so it is not mapped from other domain or xen
> itself). In case we meet a 'bad' page we'll have to split the mapping
> (and copy the page itself).

Hmmm.  Yes, we can only reassign one page at a time.  I think that
will look cleaner if you split out the reassign-or-copy into a
separate function that takes a start + order and DTRT, and then having
the loop in this function handle one p2m entry (of whatever order) per
iteration.

BTW having looked at how messy this is ending up, and how it's still
incomplete, I'd agree with Jan that resetting the domain state might
be a better approach.

Tim.

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

Re: [Xen-devel] [PATCH v8 05/13] x86: expose CBM length and COS number information

[Jan Beulich]

>>> On 21.05.15 at 10:41,  wrote:
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -216,6 +216,38 @@ void psr_ctxt_switch_to(struct domain *d)
>  }
>  }
>  
> +static int get_cat_socket_info(unsigned int socket,
> +   struct psr_cat_socket_info **info)
> +{
> +if ( !cat_socket_info )
> +return -ENODEV;
> +
> +if ( socket >= nr_sockets )
> +return -EBADSLT;
> +
> +if ( !test_bit(socket, cat_socket_enable) )
> +return -ENOENT;
> +
> +*info = cat_socket_info + socket;
> +
> +return 0;
> +}
> +
> +int psr_get_cat_l3_info(unsigned int socket, uint32_t *cbm_len,
> +uint32_t *cos_max)
> +{
> +struct psr_cat_socket_info *info;
> +int ret = get_cat_socket_info(socket, &info);
> +
> +if ( ret )
> +return ret;
> +
> +*cbm_len = info->cbm_len;
> +*cos_max = info->cos_max;
> +
> +return 0;
> +}

I doubt all supported compiler versions will be able to see that "info"
can't be used uninitialized here. Please make this explicit.

I also think this small a function shouldn't have two return points.

> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -694,6 +694,20 @@ struct xen_sysctl_pcitopoinfo {
>  typedef struct xen_sysctl_pcitopoinfo xen_sysctl_pcitopoinfo_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_sysctl_pcitopoinfo_t);
>  
> +#define XEN_SYSCTL_PSR_CAT_get_l3_info   0
> +struct xen_sysctl_psr_cat_op {
> +uint32_t cmd;   /* IN: XEN_SYSCTL_PSR_CAT_* */
> +uint32_t target;/* IN: socket to be operated on */

If this is always the socket number, why would the variable be
named anything other than "socket". If otoh subsequent patches
use it differently, I think the comment should be omitted now
rather than being dropped then (or it should be given its final
wording from the beginning).

Jan


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

Re: [Xen-devel] [PATCH v8 03/13] x86: maintain COS to CBM mapping for each socket

[Jan Beulich]

>>> On 21.05.15 at 10:41,  wrote:
> For each socket, a COS to CBM mapping structure is maintained for each
> COS. The mapping is indexed by COS and the value is the corresponding
> CBM. Different VMs may use the same CBM, a reference count is used to
> indicate if the CBM is available.
> 
> Signed-off-by: Chao Peng 
> Reviewed-by: Andrew Cooper 
> ---
> Changes in v8:
> * Move the memory allocation and CAT initialization code to CPU_UP_PREPARE.
> * Add memory freeing code in CPU_DEAD path.

Changes like this imo invalidate any tags given for earlier versions.

> +
> +*rc = 0;
> +
> +return;
> +
> +}
> +
> +

Stray return and blank lines.

> +static int cat_cpu_init(unsigned int cpu)
> +{
> +int rc;
> +const struct cpuinfo_x86 *c = cpu_data + cpu;
> +
> +if ( !cpu_has(c, X86_FEATURE_CAT) )
> +return 0;
> +
> +if ( test_bit(cpu_to_socket(cpu), cat_socket_enable) )
> +return 0;
> +
> +if ( cpu == smp_processor_id() )
> +do_cat_cpu_init(&rc);
> +else
> +on_selected_cpus(cpumask_of(cpu), do_cat_cpu_init, &rc, 1);

This now being called in the context of CPU_UP_PREPARE, I can't see
how this works at all: Neither would the CPU's cpu_data[] instance be
initialized by that time, nor would you be able to IPI that CPU, nor can I
see how the if() branch could ever get entered. Was this tested at all?

> @@ -283,14 +331,24 @@ static void psr_cpu_fini(unsigned int cpu)
>  static int cpu_callback(
>  struct notifier_block *nfb, unsigned long action, void *hcpu)
>  {
> +int rc = 0;
>  unsigned int cpu = (unsigned long)hcpu;
>  
> -if ( action == CPU_STARTING )
> -psr_cpu_init();
> -else if ( action == CPU_DEAD )
> +switch ( action )
> +{
> +case CPU_UP_PREPARE:
> +rc = psr_cpu_prepare(cpu);
> +break;
> +case CPU_STARTING:
> +psr_cpu_starting();

This not being run for the boot CPU, ...

> @@ -305,7 +363,7 @@ static int __init psr_presmp_init(void)
>  if ( opt_psr & PSR_CAT )
>  init_psr_cat();
>  
> -psr_cpu_init();
> +psr_cpu_prepare(0);
>  if ( psr_cmt_enabled() || cat_socket_info )

... don't you need to call it here too?

Jan


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

Re: [Xen-devel] [PATCH v7 04/10] xen: Introduce XEN_DOMCTL_soft_reset

[Vitaly Kuznetsov]

Tim Deegan  writes:

> At 13:56 +0200 on 28 May (1432821360), Vitaly Kuznetsov wrote:
>> Tim Deegan  writes:
>> >> +last_gmfn = domain_get_maximum_gpfn(source_d);
>> >> +gmfn = *gmfn_start;
>> >> +while ( gmfn <= last_gmfn )
>> >> +{
>> >> +page = get_page_from_gfn(source_d, gmfn, &p2mt, 0);
>> >
>> > In order to be safe against p2m changes, you should use
>> > get_gfn_type_access() _here_, and put_gfn() when you're finished with the
>> > gfn.  You'll also need to get_page() the returned MFN, of course, to
>> > make sure that it isn't freed before you reassign it.
>> 
>> The only problem I see is the fact that get_gfn_type_access() is
>> x86-specific. Actually, I don't see why we can't have
>> get_gfn_type_access() for ARM: it locks the whole p2m with gfn_lock
>> (which is just p2m_lock() on x86) and then resolves the gfn. I'm not
>> sure what should be the right approach for this series: make this
>> hypercall x86-specific (temporary before we have all the required bits
>> in ARM) or try to make something up for ARM...
>
> I think it would be best to add this call to ARM.
>
>> >> +while ( next_page && !is_xen_heap_page(next_page) &&
>> >> +page_to_mfn(next_page) == mfn + count )
>> >
>> > What's the purpose of this second loop?  It doesn't seem to be doing
>> > anything that the outer loop couldn't.
>> 
>> True. This second loops searches for a continuous region to preserve the
>> order of mappings (when possible)
>
> Ah; I think this, like the PoD case, should the more detailed p2m
> lookup to get the actual order of the current mapping.  That should be
> shorter and more readable, and probably more correct.

If we bring get_gfn_type_access() call to the top level it becomes
possible (and easy) but (if I'm not mistaken) we still need to walk
through all pages of the mapping checking that each of them has the
reqiuired count_info (so it is not mapped from other domain or xen
itself). In case we meet a 'bad' page we'll have to split the mapping
(and copy the page itself).

-- 
  Vitaly

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

Re: [Xen-devel] [PATCH v7 00/10] toolstack-based approach to pvhvm guest kexec

[Jan Beulich]

>>> On 28.05.15 at 14:27,  wrote:
> "Jan Beulich"  writes:
> 
> On 27.05.15 at 17:25,  wrote:
>>> This patch series provides x86 PVHVM domains with an ability to perform
>>> kexec/kdump-style operations.
>>
>> Before I get to look at this latest version, may I go a step back and
>> ask for clarification whether all of these (seemingly fragile)
>> manipulations are actually needed (such a rationale for the series
>> would btw be quite good to have in the overview mail, rather than
>> having to wade through mailing list history). In particular, why is it
>> that we need a new domain in the first place? After all, native
>> kexec doesn't imply a new physical machine either. Perhaps that
>> was discussed a long while back, but I can't seem to find any of
>> that now that I would like to read through it again.
> 
> Yes, here are some highlights from last year's discussion:
> 
> http://lists.xen.org/archives/html/xen-devel/2014-08/msg01908.html 
> 
> http://lists.xen.org/archives/html/xen-devel/2014-08/msg01979.html 

I.e. what you currently implement is David's model without Konrad's
later alternative really having been explored? Iiuc David's main
reservation (which I share) was against a myriad of reset-this and
reset-that hypercalls, which Konrad's reset-everything would
address equally well.

Jan


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

Re: [Xen-devel] [PATCH v8 02/13] x86: detect and initialize Intel CAT feature

[Jan Beulich]

>>> On 21.05.15 at 10:41,  wrote:
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -19,14 +19,25 @@
>  #include 
>  
>  #define PSR_CMT(1<<0)
> +#define PSR_CAT(1<<1)
> +
> +struct psr_cat_socket_info {
> +unsigned int cbm_len;
> +unsigned int cos_max;
> +};
>  
>  struct psr_assoc {
>  uint64_t val;
>  };
>  
>  struct psr_cmt *__read_mostly psr_cmt;
> +
> +static unsigned long *__read_mostly cat_socket_enable;
> +static struct psr_cat_socket_info *__read_mostly cat_socket_info;
> +
>  static unsigned int __initdata opt_psr;
>  static unsigned int __initdata opt_rmid_max = 255;
> +static unsigned int opt_cos_max = 255;

opt_* variables should generally be either __initdata or
__read_mostly (the latter in this case).

> @@ -194,16 +209,86 @@ void psr_ctxt_switch_to(struct domain *d)
>  }
>  }
>  
> +static void cat_cpu_init(void)
> +{
> +unsigned int eax, ebx, ecx, edx;
> +struct psr_cat_socket_info *info;
> +unsigned int socket;
> +unsigned int cpu = smp_processor_id();
> +const struct cpuinfo_x86 *c = cpu_data + cpu;
> +
> +if ( !cpu_has(c, X86_FEATURE_CAT) )
> +return;
> +
> +socket = cpu_to_socket(cpu);
> +if ( test_bit(socket, cat_socket_enable) )
> +return;
> +
> +cpuid_count(PSR_CPUID_LEVEL_CAT, 0, &eax, &ebx, &ecx, &edx);

While one would hope that X86_FEATURE_CAT implies the respective
CPUID leaf being available, I think explicitly checking this should still
be done just like is the case elsewhere.

> +if ( ebx & PSR_RESOURCE_TYPE_L3 )
> +{
> +cpuid_count(PSR_CPUID_LEVEL_CAT, 1, &eax, &ebx, &ecx, &edx);
> +info = cat_socket_info + socket;
> +info->cbm_len = (eax & 0x1f) + 1;
> +info->cos_max = min(opt_cos_max, edx & 0x);
> +
> +set_bit(socket, cat_socket_enable);
> +printk(XENLOG_INFO "CAT: enabled on socket %u, cos_max:%u, 
> cbm_len:%u\n",
> +   socket, info->cos_max, info->cbm_len);
> +}
> +}
> +
> +static void cat_cpu_fini(unsigned int cpu)
> +{
> +unsigned int socket = cpu_to_socket(cpu);
> +
> +if ( !socket_cpumask[socket] || cpumask_empty(socket_cpumask[socket]) )
> +clear_bit(socket, cat_socket_enable);
> +}

This being called from the CPU_DEAD notification, you now depend
on cpu_smpboot_free) to run ahead of you. Which isn't the case
afaict, and even if it happened to be that way you shouldn't rely
on it without explicitly enforcing ordering between the two by
setting the priority of on of them to a non-default value.

> +static void __init init_psr_cat(void)
> +{
> +if ( opt_cos_max < 1 )
> +{
> +printk(XENLOG_INFO "CAT: disabled, cos_max is too small\n");
> +return;
> +}

Is opt_cos_max == 1 really useful for anything?

>  static int cpu_callback(
>  struct notifier_block *nfb, unsigned long action, void *hcpu)
>  {
> +unsigned int cpu = (unsigned long)hcpu;
> +
>  if ( action == CPU_STARTING )
>  psr_cpu_init();
> +else if ( action == CPU_DEAD )
> +psr_cpu_fini(cpu);

switch()

Jan


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

Re: [Xen-devel] [PATCH v7 04/10] xen: Introduce XEN_DOMCTL_soft_reset

[Tim Deegan]

At 13:56 +0200 on 28 May (1432821360), Vitaly Kuznetsov wrote:
> Tim Deegan  writes:
> >> +last_gmfn = domain_get_maximum_gpfn(source_d);
> >> +gmfn = *gmfn_start;
> >> +while ( gmfn <= last_gmfn )
> >> +{
> >> +page = get_page_from_gfn(source_d, gmfn, &p2mt, 0);
> >
> > In order to be safe against p2m changes, you should use
> > get_gfn_type_access() _here_, and put_gfn() when you're finished with the
> > gfn.  You'll also need to get_page() the returned MFN, of course, to
> > make sure that it isn't freed before you reassign it.
> 
> The only problem I see is the fact that get_gfn_type_access() is
> x86-specific. Actually, I don't see why we can't have
> get_gfn_type_access() for ARM: it locks the whole p2m with gfn_lock
> (which is just p2m_lock() on x86) and then resolves the gfn. I'm not
> sure what should be the right approach for this series: make this
> hypercall x86-specific (temporary before we have all the required bits
> in ARM) or try to make something up for ARM...

I think it would be best to add this call to ARM.

> >> +while ( next_page && !is_xen_heap_page(next_page) &&
> >> +page_to_mfn(next_page) == mfn + count )
> >
> > What's the purpose of this second loop?  It doesn't seem to be doing
> > anything that the outer loop couldn't.
> 
> True. This second loops searches for a continuous region to preserve the
> order of mappings (when possible)

Ah; I think this, like the PoD case, should the more detailed p2m
lookup to get the actual order of the current mapping.  That should be
shorter and more readable, and probably more correct.

Cheers,

Tim.

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

Re: [Xen-devel] [PATCH] docs/xen: Clarification to terms used in hypervisor memory management

[Andrew Cooper]

On 28/05/15 13:25, Tim Deegan wrote:
> At 12:34 +0100 on 28 May (1432816489), Andrew Cooper wrote:
>> Memory management is hard[citation needed].  Furthermore, it isn't helped by
>> the inconsistent use of terms through the code, or that some terms have
>> changed meaning over time.
>>
>> Describe the currently-used terms in a more practical fashon, so new code has
>> a concrete reference.
>>
>> Signed-off-by: Andrew Cooper 
> Acked-by: Tim Deegan , with one reservation.
>
>> + * - Phase out all use of gpfn/gmfn where it pertains to pfn or mfn.
> I think you mean s/gpfn/gfn/, which I agree with.  gmfn I'm not so
> sure of.  Shadow code uses variants on gmfn/smfn to denote pairs of
> mfns (the guest's pagetable and its shadow), which I think is
> defensible.

That was my intended meaning of "where it pertains to pfn or mfn",
although I suppose I should have gone with my original sentence of
"except in the shadow code, where gmfn has a distinct meaning".

~Andrew

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

Re: [Xen-devel] [PATCH v8 01/13] x86: add socket_cpumask

[Jan Beulich]

>>> On 21.05.15 at 10:41,  wrote:
> --- a/xen/arch/x86/mpparse.c
> +++ b/xen/arch/x86/mpparse.c
> @@ -87,6 +87,18 @@ void __init set_nr_cpu_ids(unsigned int max_cpus)
>  #endif
>  }
>  
> +void __init set_nr_sockets(void)
> +{
> +unsigned int cpus = bitmap_weight(phys_cpu_present_map.mask,
> +  boot_cpu_data.x86_max_cores *
> +  boot_cpu_data.x86_num_siblings);

How did you come to this expression for the bitmap size? I.e.
why not simply physids_weight(phys_cpu_present_map)?

> +
> +if ( cpus == 0 )
> +cpus = 1;
> +
> +nr_sockets = DIV_ROUND_UP(num_processors + disabled_cpus, cpus);
> +}

Is there a reason why this can't just be added to the end of the
immediately preceding set_nr_cpu_ids()?

> @@ -638,6 +649,8 @@ static int cpu_smpboot_alloc(unsigned int cpu)
>  unsigned int order, memflags = 0;
>  nodeid_t node = cpu_to_node(cpu);
>  struct desc_struct *gdt;
> +unsigned int socket = cpu_to_socket(cpu);
> +
>  
>  if ( node != NUMA_NO_NODE )

Stray blank line being added.

> @@ -717,6 +734,12 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
>  
>  stack_base[0] = stack_start;
>  
> +socket_cpumask = xzalloc_array(cpumask_var_t, nr_sockets);
> +if ( !socket_cpumask )
> +panic("No memory for socket CPU siblings map");
> +if ( !zalloc_cpumask_var(socket_cpumask) )
> +panic("No memory for socket CPU siblings cpumask");

Please combine the two if()s to have just a single panic() invocation.
If either fails, it doesn't really matter which one it was.

> --- a/xen/include/asm-x86/smp.h
> +++ b/xen/include/asm-x86/smp.h
> @@ -58,6 +58,15 @@ int hard_smp_processor_id(void);
>  
>  void __stop_this_cpu(void);
>  
> +/*
> + * The value may be greater than the actual socket number in the system and
> + * is considered not to change from the initial startup.
> + */
> +extern unsigned int nr_sockets;

In the comment, instead of "considered" do you perhaps mean
"expected" or even "required"?

> +/* Representing HT and core siblings in each socket */
> +extern cpumask_var_t *socket_cpumask;

Comment style.

Jan


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

[Xen-devel] [xen-4.2-testing test] 57409: regressions - FAIL

[osstest service user]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xend-winxpsp3 16 guest-stop   fail REGR. vs. 53018
 test-amd64-i386-xend-qemut-winxpsp3 16 guest-stop fail REGR. vs. 53018

Regressions which are regarded as allowable (not blocking):
 test-amd64-i386-xl-qemuu-win7-amd64 12 guest-localmigrate  fail like 52651

Tests which did not succeed, but are not blocking:
 test-i386-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)   blocked n/a
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a
 build-i386-rumpuserxen5 rumpuserxen-buildfail   never pass
 build-amd64-rumpuserxen   5 rumpuserxen-buildfail   never pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64  9 debian-hvm-install fail never pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  9 debian-hvm-install  fail never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-i386-i386-libvirt   12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-win7-amd64 16 guest-stop   fail never pass
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail never pass

version targeted for testing:
 xen  63aeca00c805fa1c47d9f7b1978e83e41ab482d4
baseline version:
 xen  7e527e2ab6c95ef84035d02e9e50b956a0d469c9


People who touched revisions under test:
  Ian Jackson 
  Petr Matousek 


jobs:
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 build-amd64-rumpuserxen  fail
 build-i386-rumpuserxen   fail
 test-amd64-amd64-xl  pass
 test-amd64-i386-xl   pass
 test-i386-i386-xlpass
 test-amd64-i386-rhel6hvm-amd pass
 test-amd64-i386-qemut-rhel6hvm-amd   pass
 test-amd64-i386-qemuu-rhel6hvm-amd   pass
 test-amd64-amd64-xl-qemut-debianhvm-amd64pass
 test-amd64-i386-xl-qemut-debianhvm-amd64 pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-i386-xl-qemuu-debianhvm-amd64 pass
 test-amd64-i386-qemuu-freebsd10-amd64pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 fail
 test-amd64-i386-xl-qemuu-ovmf-amd64  fail
 test-amd64-amd64-rumpuserxen-amd64   blocked 
 test-amd64-amd64-xl-qemut-win7-amd64 pass
 test-amd64-i386-xl-qemut-win7-amd64  fail
 test-amd64-amd64-xl-qemuu-win7-amd64 fail
 test-amd64-i386-xl-qemuu-win7-amd64  fail
 test-amd64-amd64-xl-win7-amd64   fail
 test-amd64-i386-xl-win7-amd64pass
 test-amd64-amd64-xl-credit2  pass
 test-i386-i386-xl-credit2pass
 test-amd64-i386-qemuu-freebsd10-i386 pass
 test-amd64-i386-rumpuserxen-i386 blocked 
 test-i386-i386-rumpuserxen-i386  blocked 
 test-amd64-i386-rhel6hvm-intel   pass
 test-amd64-i386-qemut-rhel6hvm-intel pass
 test-amd64-i386-qemuu-rhel6hvm-intel pass
 test-amd64-amd64-libvirt pass
 test-amd64-i386-libvirt  pass
 test-i386-i386-libvirt   pass
 test-amd64-amd64-xl-multivcpupass
 test-i386-i386-xl-multivcpu  pass
 test-amd64-amd64-pairpass
 test-amd64-i386-pair   

Re: [Xen-devel] [RFC][v2][PATCH 05/14] xen/x86/p2m: introduce set_identity_p2m_entry

[Jan Beulich]

>>> On 22.05.15 at 11:35,  wrote:
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -898,6 +898,36 @@ int set_mmio_p2m_entry(struct domain *d, unsigned long 
> gfn, mfn_t mfn,
>  return set_typed_p2m_entry(d, gfn, mfn, p2m_mmio_direct, access);
>  }
>  
> +int set_identity_p2m_entry(struct domain *d, unsigned long gfn,
> +   p2m_access_t p2ma)
> +{
> +p2m_type_t p2mt;
> +p2m_access_t a;
> +mfn_t mfn;
> +struct p2m_domain *p2m = p2m_get_hostp2m(d);
> +int ret = -EBUSY;
> +
> +gfn_lock(p2m, gfn, 0);
> +
> +mfn = p2m->get_entry(p2m, gfn, &p2mt, &a, 0, NULL);
> +
> +if ( p2mt == p2m_invalid )
> +ret = p2m_set_entry(p2m, gfn, _mfn(gfn), PAGE_ORDER_4K,
> +p2m_mmio_direct, p2ma);
> +else if ( mfn_x(mfn) == gfn && p2mt == p2m_mmio_direct && a == p2ma )
> +ret = 0;
> +else
> +{
> +printk(XENLOG_G_WARNING
> +   "Cannot identity map d%d:%lx, already mapped to %lx.\n",
> +   d->domain_id, gfn, mfn_x(mfn));
> +}

With the redundant braces here dropped or the ret = -EBUSY moved
into this block,
Reviewed-by: Jan Beulich 

I also reduced the Cc list quite significantly - I don't understand why
so many people were Cc-ed on this patch.

Jan


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

Re: [Xen-devel] [PATCH v7 00/10] toolstack-based approach to pvhvm guest kexec

[Vitaly Kuznetsov]

"Jan Beulich"  writes:

 On 27.05.15 at 17:25,  wrote:
>> This patch series provides x86 PVHVM domains with an ability to perform
>> kexec/kdump-style operations.
>
> Before I get to look at this latest version, may I go a step back and
> ask for clarification whether all of these (seemingly fragile)
> manipulations are actually needed (such a rationale for the series
> would btw be quite good to have in the overview mail, rather than
> having to wade through mailing list history). In particular, why is it
> that we need a new domain in the first place? After all, native
> kexec doesn't imply a new physical machine either. Perhaps that
> was discussed a long while back, but I can't seem to find any of
> that now that I would like to read through it again.

Yes, here are some highlights from last year's discussion:

http://lists.xen.org/archives/html/xen-devel/2014-08/msg01908.html

http://lists.xen.org/archives/html/xen-devel/2014-08/msg01979.html

http://lists.xen.org/archives/html/xen-devel/2014-08/msg01918.html

-- 
  Vitaly

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

Re: [Xen-devel] [PATCH] docs/xen: Clarification to terms used in hypervisor memory management

[Tim Deegan]

At 12:34 +0100 on 28 May (1432816489), Andrew Cooper wrote:
> Memory management is hard[citation needed].  Furthermore, it isn't helped by
> the inconsistent use of terms through the code, or that some terms have
> changed meaning over time.
> 
> Describe the currently-used terms in a more practical fashon, so new code has
> a concrete reference.
> 
> Signed-off-by: Andrew Cooper 

Acked-by: Tim Deegan , with one reservation.

> + * - Phase out all use of gpfn/gmfn where it pertains to pfn or mfn.

I think you mean s/gpfn/gfn/, which I agree with.  gmfn I'm not so
sure of.  Shadow code uses variants on gmfn/smfn to denote pairs of
mfns (the guest's pagetable and its shadow), which I think is
defensible.

Tim.


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

Re: [Xen-devel] [PATCH v2 26/41] arm : acpi add xen environment table

[Jan Beulich]

>>> On 28.05.15 at 14:12,  wrote:
> On Thu, 28 May 2015, Jan Beulich wrote:
>> >>> On 28.05.15 at 12:58,  wrote:
>> > Let's take a closer look at this table. After the boilierplate, these
>> > are the interesting fields:
>> > 
>> > GNT Start, GNT Size
>> > Evtchn Intr, Evtchn Intr Flags
>> > 
>> > After the table, it is clearly stated:
>> > 
>> > "The Grant Table region is optional."
>> > 
>> > and
>> > 
>> > "The Event Channel Interrupt is optional."
>> > 
>> > So I think there is no problem: we don't want to pass any info in that
>> > table? Sure, let's not pass any. We can still use it to flag the
>> > presence of Xen on the platform.
>> 
>> Even more so a reason to use what base ACPI has, without any
>> custom table.
> 
> Could you please make a concrete suggestion with table names and fields?

I already pointed you at 6.0's new FADT field "Hypervisor Vendor
Identity".

Jan


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

Re: [Xen-devel] [PATCH v7 00/10] toolstack-based approach to pvhvm guest kexec

[Jan Beulich]

>>> On 27.05.15 at 17:25,  wrote:
> This patch series provides x86 PVHVM domains with an ability to perform
> kexec/kdump-style operations.

Before I get to look at this latest version, may I go a step back and
ask for clarification whether all of these (seemingly fragile)
manipulations are actually needed (such a rationale for the series
would btw be quite good to have in the overview mail, rather than
having to wade through mailing list history). In particular, why is it
that we need a new domain in the first place? After all, native
kexec doesn't imply a new physical machine either. Perhaps that
was discussed a long while back, but I can't seem to find any of
that now that I would like to read through it again.

Jan


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

Re: [Xen-devel] [PATCH v2 26/41] arm : acpi add xen environment table

[Stefano Stabellini]

On Thu, 28 May 2015, Jan Beulich wrote:
> >>> On 28.05.15 at 12:58,  wrote:
> > Let's take a closer look at this table. After the boilierplate, these
> > are the interesting fields:
> > 
> > GNT Start, GNT Size
> > Evtchn Intr, Evtchn Intr Flags
> > 
> > After the table, it is clearly stated:
> > 
> > "The Grant Table region is optional."
> > 
> > and
> > 
> > "The Event Channel Interrupt is optional."
> > 
> > So I think there is no problem: we don't want to pass any info in that
> > table? Sure, let's not pass any. We can still use it to flag the
> > presence of Xen on the platform.
> 
> Even more so a reason to use what base ACPI has, without any
> custom table.

Could you please make a concrete suggestion with table names and fields?

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

Re: [Xen-devel] [PATCH V7] xen/vm_event: Clean up control-register-write vm_events and add XCR0 event

[Jan Beulich]

>>> On 28.05.15 at 12:56,  wrote:
> On 05/28/2015 11:39 AM, Jan Beulich wrote:
> On 28.05.15 at 07:33,  wrote:
>>> Changes since V6:
>>>  - Removed "Reviewed-by: Tim Deegan ", as the patch
>>>went beyond cosmetic-only changes.
>>>  - Parenthesized hvm_event_cr in event.c, to prevent expansion of
>>>the macro with the same name if event.h gets #included.
>> 
>> I'm sorry, but I specifically said that you need to include the header
>> declaring the function (and macro) in the file defining it, but ...
>> 
>>> --- a/xen/arch/x86/hvm/event.c
>>> +++ b/xen/arch/x86/hvm/event.c
>>> @@ -21,6 +21,7 @@
>>>  
>>>  #include 
>>>  #include 
>>> +#include 
>>>  #include 
>>>  
>>>  static void hvm_event_fill_regs(vm_event_request_t *req)
>> 
>> ... you didn't (and you also didn't in a prereq patch, which would have
>> been the alternative).
> 
> I see, sorry for the misunderstanding - I read your comment to mean that
> in case the header gets included at a later time there might be
> problems, which now won't happen. I will re-submit with the proper
> header included.

Thanks. Just to clarify: What I said was that your previous version
introduced a latent issue, hidden by another bug. The implied
consequence for someone like me was that both issues should be
fixed, rather than waiting for either to become a real one. Perhaps
I should have said so explicitly...

Jan


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

Re: [Xen-devel] [PATCH v2 26/41] arm : acpi add xen environment table

[Jan Beulich]

>>> On 28.05.15 at 12:58,  wrote:
> Let's take a closer look at this table. After the boilierplate, these
> are the interesting fields:
> 
> GNT Start, GNT Size
> Evtchn Intr, Evtchn Intr Flags
> 
> After the table, it is clearly stated:
> 
> "The Grant Table region is optional."
> 
> and
> 
> "The Event Channel Interrupt is optional."
> 
> So I think there is no problem: we don't want to pass any info in that
> table? Sure, let's not pass any. We can still use it to flag the
> presence of Xen on the platform.

Even more so a reason to use what base ACPI has, without any
custom table.

Jan


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

Re: [Xen-devel] [PATCH v7 04/10] xen: Introduce XEN_DOMCTL_soft_reset

[Vitaly Kuznetsov]

Tim Deegan  writes:

> Hi,
>
> At 17:25 +0200 on 27 May (1432747540), Vitaly Kuznetsov wrote:
>> New operation reassigns all memory pages from source domain to the 
>> destination
>> domain mapping them at exactly the same GFNs. Pages mapped more than once 
>> (e.g.
>> grants) are being copied.
>> 
>> Signed-off-by: Vitaly Kuznetsov 
>> ---
>> Changes in v7:
>> - is_soft_reset flag added to struct domain to preserve destination domain's
>>   paused state across possible hypercall preemption.
>> [Jan Beulich]
>> - XENMEM_soft_reset -> XEN_DOMCTL_soft_reset
>> - domain_soft_reset() returns int
>> - no locking for is_dying as it is now proteced by domctl_lock
>> - print a warning on !mfn_valid case
>> - check PGC_allocated for pages
>> - no print on assign_pages failure (it prints error messages on both its 
>> failure paths)
>> - don't re-read page->count_info, use copy_page flag
>> - minor stucturing change
>> - pause both source and destination domain while processing the hypercall
>> - remove nr_transferred from public interface
>> [Tim Deegan]
>> - use get_gfn_type_access() in PoD case (x86-only)
>> ---
>>  xen/common/domain.c | 186 
>> 
>>  xen/common/domctl.c |  72 +
>>  xen/include/public/domctl.h |  28 +++
>>  xen/include/xen/sched.h |   6 ++
>>  4 files changed, 292 insertions(+)
>> 
>> diff --git a/xen/common/domain.c b/xen/common/domain.c
>> index 7825c56..824f325 100644
>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -1006,6 +1006,192 @@ int domain_unpause_by_systemcontroller(struct domain 
>> *d)
>>  return 0;
>>  }
>>  
>> +int domain_soft_reset(struct domain *source_d, struct domain *dest_d,
>> +  xen_pfn_t *gmfn_start)
>> +{
>> +int rc = 0;
>> +unsigned long mfn, mfn_new, gmfn, last_gmfn, count;
>> +unsigned int order;
>> +p2m_type_t p2mt;
>> +struct page_info *page, *new_page, *next_page;
>> +int drop_dom_ref, copy_page;
>> +
>> +last_gmfn = domain_get_maximum_gpfn(source_d);
>> +gmfn = *gmfn_start;
>> +while ( gmfn <= last_gmfn )
>> +{
>> +page = get_page_from_gfn(source_d, gmfn, &p2mt, 0);
>
> In order to be safe against p2m changes, you should use
> get_gfn_type_access() _here_, and put_gfn() when you're finished with the
> gfn.  You'll also need to get_page() the returned MFN, of course, to
> make sure that it isn't freed before you reassign it.

The only problem I see is the fact that get_gfn_type_access() is
x86-specific. Actually, I don't see why we can't have
get_gfn_type_access() for ARM: it locks the whole p2m with gfn_lock
(which is just p2m_lock() on x86) and then resolves the gfn. I'm not
sure what should be the right approach for this series: make this
hypercall x86-specific (temporary before we have all the required bits
in ARM) or try to make something up for ARM...

>> +if ( unlikely(page == NULL) )
>> +{
>> +#ifdef CONFIG_X86
>> +struct p2m_domain *p2m = p2m_get_hostp2m(source_d);
>> +p2m_access_t p2ma;
>> +mfn_t mfn_p2m;
>> +
>> +order = 0;
>> +mfn_p2m = get_gfn_type_access(p2m, gmfn,
>> +  &p2mt, &p2ma, 0, &order);
>> +if ( p2m_is_pod(p2mt) )
>> +{
>> +rc = guest_physmap_mark_populate_on_demand(dest_d, gmfn,
>> +   order);
>> +if ( unlikely(rc) )
>> +{
>> +printk(XENLOG_G_ERR "Failed to mark Dom%d's GFN %lx"
>> +   " (order: %d) as PoD\n", source_d->domain_id,
>> +   gmfn, order);
>> +goto fail;
>> +}
>> +}
>> +put_gfn(source_d, gmfn);
>> +gmfn += 1ul << order;
>> +#else
>> +gmfn++;
>> +#endif
>> +goto preempt_check;
>> +}
>> +
>> +mfn = page_to_mfn(page);
>> +if ( unlikely(!mfn_valid(mfn)) )
>> +{
>> +printk(XENLOG_G_WARNING "Dom%d's GFN %lx points to invalid 
>> MFN\n",
>> +   source_d->domain_id, gmfn);
>> +put_page(page);
>> +gmfn++;
>> +goto preempt_check;
>> +}
>> +
>> +next_page = page;
>> +count = 0;
>> +copy_page = 0;
>> +while ( next_page && !is_xen_heap_page(next_page) &&
>> +page_to_mfn(next_page) == mfn + count )
>
> What's the purpose of this second loop?  It doesn't seem to be doing
> anything that the outer loop couldn't.

True. This second loops searches for a continuous region to preserve the
order of mappings (when possible) but as it advances the gmfn it doesn't
bring any performance penalty. I was under an impression it makes this
code easier to read but if you think it doesn't I won't object.

Thanks,

-- 
  Vitaly

[Xen-devel] [PATCH] docs/xen: Clarification to terms used in hypervisor memory management

[Andrew Cooper]

Memory management is hard[citation needed].  Furthermore, it isn't helped by
the inconsistent use of terms through the code, or that some terms have
changed meaning over time.

Describe the currently-used terms in a more practical fashon, so new code has
a concrete reference.

Signed-off-by: Andrew Cooper 
CC: Keir Fraser 
CC: Jan Beulich 
CC: Tim Deegan 
CC: Ian Campbell 
CC: Stefano Stabellini 

---
v2: Adjustments following review.
---
 xen/include/xen/mm.h |   57 +++---
 1 file changed, 36 insertions(+), 21 deletions(-)

diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index a066363..7dc580f 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -1,28 +1,43 @@
 /**
  * include/xen/mm.h
- * 
+ *
  * Definitions for memory pages, frame numbers, addresses, allocations, etc.
- * 
- * Note that Xen must handle several different physical 'address spaces' and
- * there is a consistent terminology for these:
- * 
- * 1. gpfn/gpaddr: A guest-specific pseudo-physical frame number or address.
- * 2. gmfn/gmaddr: A machine address from the p.o.v. of a particular guest.
- * 3. mfn/maddr:   A real machine frame number or address.
- * 4. pfn/paddr:   Used in 'polymorphic' functions that work across all
- * address spaces, depending on context. See the pagetable
- * conversion macros in asm-x86/page.h for examples.
- * Also 'paddr_t' is big enough to store any physical address.
- * 
- * This scheme provides consistent function and variable names even when
- * different guests are running in different memory-management modes.
- * 1. A guest running in auto-translated mode (e.g., shadow_mode_translate())
- *will have gpfn == gmfn and gmfn != mfn.
- * 2. A paravirtualised x86 guest will have gpfn != gmfn and gmfn == mfn.
- * 3. A paravirtualised guest with no pseudophysical overlay will have
- *gpfn == gpmfn == mfn.
- * 
+ *
  * Copyright (c) 2002-2006, K A Fraser 
+ *
+ * +-+
+ *  Xen Memory Management
+ * +-+
+ *
+ * Xen has to handle many different address spaces.  It is important not to
+ * get these spaces mixed up.  The following is a consistent terminology which
+ * should be adhered to.
+ *
+ * mfn: Machine Frame Number
+ *   The values Xen puts into its own pagetables.  This is the host physical
+ *   memory address space with RAM, MMIO etc.
+ *
+ * gfn: Guest Frame Number
+ *   The values a guest puts in its own pagetables.  For an auto-translated
+ *   guest (hardware assisted with 2nd stage translation, or shadowed), gfn !=
+ *   mfn.  For a non-translated guest which is aware of Xen, gfn == mfn.
+ *
+ * pfn: Pseudophysical Frame Number
+ *   A linear idea of a guest physical address space. For an auto-translated
+ *   guest, pfn == gfn while for a non-translated guest, pfn != gfn.
+ *
+ * WARNING: Some of these terms have changed over time while others have been
+ * used inconsistently, meaning that a lot of existing code does not match the
+ * definitions above.  New code should use these terms as described here, and
+ * over time older code should be corrected to be consistent.
+ *
+ * An incomplete list of larger work area:
+ * - Phase out the use of 'pfn' from the x86 pagetable code.  Callers should
+ *   know explicitly whether they are talking about mfns or gfns.
+ * - Phase out the use of 'pfn' from the ARM mm code.  A cursory glance
+ *   suggests that 'mfn' and 'pfn' are currently used interchangeably, where
+ *   'mfn' is the appropriate term to use.
+ * - Phase out all use of gpfn/gmfn where it pertains to pfn or mfn.
  */
 
 #ifndef __XEN_MM_H__
-- 
1.7.10.4


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

Re: [Xen-devel] [PATCH v2] xen: cpupools: avoid crashing if shutting down with free CPUs

[Dario Faggioli]

On Thu, 2015-05-28 at 10:03 +0100, Jan Beulich wrote:

> Could one or both of you confirm that this as well as the other patch
> it was originally paired with ("cpupool: assigning a CPU to a pool can
> fail") ought to be backported (presumably to both 4.5 and 4.4)
> please?
> 
Right, I was about to ask for this.

In fact, I've just checked, and both 4.5 and 4.4 are affected by the
issue fixed by this patch.

"cpupool: assigning a CPU to a pool can fail" should be backported as
well, IMO.

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
http://lists.xen.org/xen-devel

Re: [Xen-devel] [libvirt] [PATCH 0/4] libxl: support SPICE graphics

[Michal Privoznik]

On 09.05.2015 00:31, Jim Fehlig wrote:
> This series provides support for SPICE graphics in the libxl driver.
> The first patch is mostly code movement.  The second patch is a trivial
> name change of a structure member.  Patch3 populates the
> libxl_domain_build_info struct with SPICE info.  SPICE isn't as nice
> without QXL, so patch4 provides support for QXL video device.
> 
> Jim Fehlig (4):
>   libxl: populate build_info vfb in separate function
>   libxl: change reservedVNCPorts to reservedGraphicsPorts
>   libxl: support SPICE graphics for HVM domains
>   libxl: support QXL video device
> 
>  src/libxl/libxl_conf.c   | 150 
> +--
>  src/libxl/libxl_conf.h   |   2 +-
>  src/libxl/libxl_domain.c |   8 ++-
>  src/libxl/libxl_driver.c |   4 +-
>  4 files changed, 128 insertions(+), 36 deletions(-)
> 

ACK to all, but please see my comment to 3/4 before pushing.

Even though we are in freeze, this has been lying around for a while
therefore I think it's safe to push these.

Michal

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

Re: [Xen-devel] [PATCH v2 26/41] arm : acpi add xen environment table

[Stefano Stabellini]

On Wed, 27 May 2015, Jan Beulich wrote:
> >>> On 26.05.15 at 19:34,  wrote:
> > On Thu, 21 May 2015, Jan Beulich wrote:
> >> >>> On 21.05.15 at 12:34,  wrote:
> >> > ACPI 6.0 has been released few months after Parth and Naresh began to
> >> > implement ACPI for Xen. We could take advantage of this new field.
> >> 
> >> If at all possible - yes please, in favor of any custom tables.
> > 
> > Let me give you some more context.
> > 
> > As Julien pointed out, neither the "cpuid" like mechanism nor the
> > existing ACPI tables seemed to fit the bill, so we thought that the best
> > way forward was to simply add one more ACPI table to advertise the
> > presence of Xen on the platform.
> > 
> > Since we have a table in our hands, we might as well use it to pass some
> > useful information, specifically we thought that the existing
> > information passed via device tree would be a good place to start. This
> > is how we got to the XENV table that we have now.
> > 
> > In general I think that passing information via tables or data trees,
> > that are made for this purpose, is nicer compared to using hypercalls.
> 
> There are certainly up and down sides to each of them. For ACPI
> tables, I'm particularly puzzled by the need to re-write XSDT/RSDT
> if you want to add a table of your own to what came from firmware.
> Which in turn requires the EFI configuration table to be modified or
> replaced. Doable, but I doubt this is a very clean approach.

We already have to do stuff like that on ARM for other things too,
for example we modify the GIC and timer tables.


> If this was only for DomU (where the tables gets crafted from
> scratch anyway) it would be a different thing. But my understanding
> so far was that this is particularly also (if not exclusively) for Dom0.

Yes, your understanding is correct.


> > Regarding stable vs non-stable, the discussion is off point. What
> > matters is the version number. We can implement the current version of
> > the spec, v0.2, or we can make changes to it, introduce a new version
> > and implement that one instead. We can pick any version number we like.
> 
> Prior to a first consumer implementation - yes. But afterwards you
> need to remain backwards compatible.

But this would be the same as for the hypercall, right? Except that I
guess it is easier to version a table than an hypercall.


> > I am biased because I contributed to the current spec, so I think that
> > the version we have is reasonable, but if we want to change it that's
> > also OK for me.  In fact if you like it, I guess we could try to make it
> > arch-independent and use it on x86 too.
> 
> As you may already have guessed from my earlier response - I'd rather
> not, unless unavoidable.

Sure, we don't have to do the same thing here between x86 and ARM here.
For Xen on ARM we have been trying to export a set of ACPI tables that
actually matches the combination of virtual or physical hardware that
Dom0 is seeing. On x86 things are different.


Let's take a closer look at this table. After the boilierplate, these
are the interesting fields:

GNT Start, GNT Size
Evtchn Intr, Evtchn Intr Flags

After the table, it is clearly stated:

"The Grant Table region is optional."

and

"The Event Channel Interrupt is optional."

So I think there is no problem: we don't want to pass any info in that
table? Sure, let's not pass any. We can still use it to flag the
presence of Xen on the platform.

If we agree to that, we can move on to discussing whether we prefer to
pass the grant table region and evtchn interrupt via XENV or another
mechanism.

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

Re: [Xen-devel] [PATCH V7] xen/vm_event: Clean up control-register-write vm_events and add XCR0 event

[Razvan Cojocaru]

On 05/28/2015 11:39 AM, Jan Beulich wrote:
 On 28.05.15 at 07:33,  wrote:
>> Changes since V6:
>>  - Removed "Reviewed-by: Tim Deegan ", as the patch
>>went beyond cosmetic-only changes.
>>  - Parenthesized hvm_event_cr in event.c, to prevent expansion of
>>the macro with the same name if event.h gets #included.
> 
> I'm sorry, but I specifically said that you need to include the header
> declaring the function (and macro) in the file defining it, but ...
> 
>> --- a/xen/arch/x86/hvm/event.c
>> +++ b/xen/arch/x86/hvm/event.c
>> @@ -21,6 +21,7 @@
>>  
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  
>>  static void hvm_event_fill_regs(vm_event_request_t *req)
> 
> ... you didn't (and you also didn't in a prereq patch, which would have
> been the alternative).

I see, sorry for the misunderstanding - I read your comment to mean that
in case the header gets included at a later time there might be
problems, which now won't happen. I will re-submit with the proper
header included.


Thanks,
Razvan

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

[Xen-devel] [3.16.y-ckt stable] Patch "config: Enable NEED_DMA_MAP_STATE by default when SWIOTLB is selected" has been added to staging queue

[Luis Henriques]

This is a note to let you know that I have just added a patch titled

config: Enable NEED_DMA_MAP_STATE by default when SWIOTLB is selected

to the linux-3.16.y-queue branch of the 3.16.y-ckt extended stable tree 
which can be found at:

http://kernel.ubuntu.com/git/ubuntu/linux.git/log/?h=linux-3.16.y-queue

This patch is scheduled to be released in version 3.16.7-ckt13.

If you, or anyone else, feels it should not be added to this tree, please 
reply to this email.

For more information about the 3.16.y-ckt tree, see
https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable

Thanks.
-Luis

--

>From 13242f9325fc927df3d3f32b0b5685bf419aa994 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk 
Date: Fri, 17 Apr 2015 15:04:48 -0400
Subject: config: Enable NEED_DMA_MAP_STATE by default when SWIOTLB is selected

commit a6dfa128ce5c414ab46b1d690f7a1b8decb8526d upstream.

A huge amount of NIC drivers use the DMA API, however if
compiled under 32-bit an very important part of the DMA API can
be ommitted leading to the drivers not working at all
(especially if used with 'swiotlb=force iommu=soft').

As Prashant Sreedharan explains it: "the driver [tg3] uses
DEFINE_DMA_UNMAP_ADDR(), dma_unmap_addr_set() to keep a copy of
the dma "mapping" and dma_unmap_addr() to get the "mapping"
value. On most of the platforms this is a no-op, but ... with
"iommu=soft and swiotlb=force" this house keeping is required,
... otherwise we pass 0 while calling pci_unmap_/pci_dma_sync_
instead of the DMA address."

As such enable this even when using 32-bit kernels.

Reported-by: Ian Jackson 
Signed-off-by: Konrad Rzeszutek Wilk 
Acked-by: David S. Miller 
Acked-by: Prashant Sreedharan 
Cc: Borislav Petkov 
Cc: H. Peter Anvin 
Cc: Linus Torvalds 
Cc: Michael Chan 
Cc: Thomas Gleixner 
Cc: boris.ostrov...@oracle.com
Cc: casca...@linux.vnet.ibm.com
Cc: david.vra...@citrix.com
Cc: sanje...@broadcom.com
Cc: siva.kal...@broadcom.com
Cc: vyasev...@gmail.com
Cc: xen-de...@lists.xensource.com
Link: http://lkml.kernel.org/r/20150417190448.ga9...@l.oracle.com
Signed-off-by: Ingo Molnar 
Cc: Ben Hutchings 
Signed-off-by: Luis Henriques 
---
 arch/x86/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 12c8ea635f4d..04fe4a66c3ec 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -164,7 +164,7 @@ config SBUS

 config NEED_DMA_MAP_STATE
def_bool y
-   depends on X86_64 || INTEL_IOMMU || DMA_API_DEBUG
+   depends on X86_64 || INTEL_IOMMU || DMA_API_DEBUG || SWIOTLB

 config NEED_SG_DMA_LENGTH
def_bool y

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

Re: [Xen-devel] [PATCH OSSTEST 0/5] Debian Jessie patches

[Ian Campbell]

On Wed, 2015-05-20 at 18:56 +0100, Wei Liu wrote:
> I worked a little bit on upgrading Osstest to Jessie a few months ago but
> dropped the ball. Here are the patches I wrote.

With the recent change to support submenus I find I now need the below,
I think update-grub on Jessie must include some sort of
whitespace/reindent functionality compared with Wheezy.

8>-

>From 98079d6eb0a93c12e5e65c999bf87b5364b300bc Mon Sep 17 00:00:00 2001
From: Ian Campbell 
Date: Thu, 28 May 2015 09:24:51 +0100
Subject: [PATCH] Debian: grub2: Allow submenu and menuentry items to be
 indented.

Signed-off-by: Ian Campbell 
---
 Osstest/Debian.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index 0bf108b..16ea68b 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -444,12 +444,12 @@ sub setupboot_grub2 () {
 if (m/^function.*\{/) {
 $entry= { StartLine => $. };
 }
-if (m/^menuentry\s+[\'\"](.*)[\'\"].*\{\s*$/) {
+if (m/^\s*menuentry\s+[\'\"](.*)[\'\"].*\{\s*$/) {
 die $entry->{StartLine} if $entry;
 $entry= { Title => $1, StartLine => $., MenuEntryPath => join 
">", @offsets };
 $offsets[$#offsets]++;
 }
-if (m/^submenu\s+[\'\"](.*)[\'\"].*\{\s*$/) {
+if (m/^\s*submenu\s+[\'\"](.*)[\'\"].*\{\s*$/) {
 $submenu={ StartLine =>$., MenuEntryPath => join ">", @offsets 
};
 push @offsets,(0);
 }
-- 
2.1.4




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

Re: [Xen-devel] [osstest test] 57401: tolerable FAIL - PUSHED

[Ian Campbell]

On Thu, 2015-05-28 at 04:49 +, osstest service user wrote:
> flight 57401 osstest real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/57401/
> 
> Failures :-/ but no regressions.

Lontao, Robert,

The grub related bits of the nested series are now in the production
osstest instance.

I'm sorry but I've had a couple of high priority interrupts wrt
reviewing the remainder.

Ian.


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

Re: [Xen-devel] [PATCH V4 8/8] xen/arm: make domain_max_vcpus return value according to vGIC version

[Andrew Cooper]

On 28/05/15 11:15, Chen Baozi wrote:
> From: Chen Baozi 
>
> When a guest uses vGICv2, the maximum number of vCPU it can support
> should not be as many as MAX_VIRT_CPUS, which is 128 at the moment.
> So the domain_max_vcpus should return the value according to the vGIC
> version the domain uses.
>
> We didn't keep it as the old static inline form because it will break
> compilation when access the member of struct domain:
>
> In file included from xen/include/xen/domain.h:6:0,
>  from xen/include/xen/sched.h:10,
>  from arm64/asm-offsets.c:10:
> xen/include/asm/domain.h: In function ‘domain_max_vcpus’:
> xen/include/asm/domain.h:266:10: error: dereferencing pointer to incomplete 
> type
>  if (d->arch.vgic.version == GIC_V2)
>   ^
>
> Signed-off-by: Chen Baozi 

Looks much better!

It is a shame about the compilation issue - I had a similar issue on the
x86 side.  At some point, I need to untangle the rats nest which is our
currently header file layout, so that we don't have to rely on tricks
like this to keep it compiling.

~Andrew

> ---
>  xen/arch/arm/domain.c| 6 ++
>  xen/include/asm-arm/domain.h | 5 +
>  xen/include/asm-arm/gic.h| 3 +++
>  3 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 0cf147c..78b77b1 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -890,6 +890,12 @@ void vcpu_block_unless_event_pending(struct vcpu *v)
>  vcpu_unblock(current);
>  }
>  
> +unsigned int domain_max_vcpus(const struct domain *d)
> +{
> +return ((d->arch.vgic.version == GIC_V2) ?
> +GICV2_MAX_CPUS : GICV3_MAX_CPUS);
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
> index 603a20b..c4cb15d 100644
> --- a/xen/include/asm-arm/domain.h
> +++ b/xen/include/asm-arm/domain.h
> @@ -261,10 +261,7 @@ struct arch_vcpu
>  void vcpu_show_execution_state(struct vcpu *);
>  void vcpu_show_registers(const struct vcpu *);
>  
> -static inline unsigned int domain_max_vcpus(const struct domain *d)
> -{
> -return MAX_VIRT_CPUS;
> -}
> +unsigned int domain_max_vcpus(const struct domain *);
>  
>  /*
>   * Due to the restriction of GICv3, the number of vCPUs in AFF0 is
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index 463fffb..a7bc0d1 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -18,6 +18,9 @@
>  #ifndef __ASM_ARM_GIC_H__
>  #define __ASM_ARM_GIC_H__
>  
> +#define GICV2_MAX_CPUS  8
> +#define GICV3_MAX_CPUS  128
> +
>  #define NR_GIC_LOCAL_IRQS  NR_LOCAL_IRQS
>  #define NR_GIC_SGI 16
>  #define MAX_RDIST_COUNT4


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

[Xen-devel] [PATCH V4 4/8] xen/arm: Use cpumask_t type for vcpu_mask in vgic_to_sgi

[Chen Baozi]

From: Chen Baozi 

Use cpumask_t instead of unsigned long which can only express 64 cpus at
the most. Add the {gicv2|gicv3}_sgir_to_cpumask in corresponding vGICs
to translate GICD_SGIR/ICC_SGI1R_EL1 to vcpu_mask for vgic_to_sgi.

Signed-off-by: Chen Baozi 
---
 xen/arch/arm/vgic-v2.c | 16 ++--
 xen/arch/arm/vgic-v3.c | 19 ---
 xen/arch/arm/vgic.c| 15 +++
 xen/include/asm-arm/vgic.h |  2 +-
 4 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index 3be1a51..2dbe371 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -33,6 +33,17 @@
 #include 
 #include 
 
+static void gicv2_sgir_to_cpumask(cpumask_t *cpumask, const register_t sgir)
+{
+unsigned long target_list;
+int cpuid;
+
+target_list = ((sgir & GICD_SGI_TARGET_MASK) >> GICD_SGI_TARGET_SHIFT);
+for_each_set_bit( cpuid, &target_list, 8 )
+cpumask_set_cpu(cpuid, cpumask);
+
+}
+
 static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
 {
 struct hsr_dabt dabt = info->dabt;
@@ -201,11 +212,12 @@ static int vgic_v2_to_sgi(struct vcpu *v, register_t sgir)
 int virq;
 int irqmode;
 enum gic_sgi_mode sgi_mode;
-unsigned long vcpu_mask = 0;
+cpumask_t vcpu_mask;
 
+cpumask_clear(&vcpu_mask);
 irqmode = (sgir & GICD_SGI_TARGET_LIST_MASK) >> GICD_SGI_TARGET_LIST_SHIFT;
 virq = (sgir & GICD_SGI_INTID_MASK);
-vcpu_mask = (sgir & GICD_SGI_TARGET_MASK) >> GICD_SGI_TARGET_SHIFT;
+gicv2_sgir_to_cpumask(&vcpu_mask, sgir);
 
 /* Map GIC sgi value to enum value */
 switch ( irqmode )
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index ef9a71a..0da031c 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -972,17 +972,30 @@ write_ignore:
 return 1;
 }
 
+static void gicv3_sgir_to_cpumask(cpumask_t *cpumask, const register_t sgir)
+{
+int target, cpuid;
+unsigned long target_mask = sgir & ICH_SGI_TARGETLIST_MASK;
+
+for_each_set_bit( target, &target_mask, 16 )
+{
+/* XXX: We assume that only AFF1 is used in ICC_SGI1R_EL1. */
+cpuid = target + ((sgir >> 16) & 0xff) * 16;
+cpumask_set_cpu(cpuid, cpumask);
+}
+}
+
 static int vgic_v3_to_sgi(struct vcpu *v, register_t sgir)
 {
 int virq;
 int irqmode;
 enum gic_sgi_mode sgi_mode;
-unsigned long vcpu_mask = 0;
+cpumask_t vcpu_mask;
 
+cpumask_clear(&vcpu_mask);
 irqmode = (sgir >> ICH_SGI_IRQMODE_SHIFT) & ICH_SGI_IRQMODE_MASK;
 virq = (sgir >> ICH_SGI_IRQ_SHIFT ) & ICH_SGI_IRQ_MASK;
-/* SGI's are injected at Rdist level 0. ignoring affinity 1, 2, 3 */
-vcpu_mask = sgir & ICH_SGI_TARGETLIST_MASK;
+gicv3_sgir_to_cpumask(&vcpu_mask, sgir);
 
 /* Map GIC sgi value to enum value */
 switch ( irqmode )
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 7b387b7..4bf8486 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -318,9 +318,8 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
 }
 }
 
-/* TODO: unsigned long is used to fit vcpu_mask.*/
 int vgic_to_sgi(struct vcpu *v, register_t sgir, enum gic_sgi_mode irqmode, 
int virq,
-unsigned long vcpu_mask)
+cpumask_t vcpu_mask)
 {
 struct domain *d = v->domain;
 int vcpuid;
@@ -341,12 +340,12 @@ int vgic_to_sgi(struct vcpu *v, register_t sgir, enum 
gic_sgi_mode irqmode, int
  * SGI_TARGET_SELF mode. So Force vcpu_mask to 0
  */
 perfc_incr(vgic_sgi_others);
-vcpu_mask = 0;
+cpumask_clear(&vcpu_mask);
 for ( i = 0; i < d->max_vcpus; i++ )
 {
 if ( i != current->vcpu_id && d->vcpu[i] != NULL &&
  is_vcpu_online(d->vcpu[i]) )
-set_bit(i, &vcpu_mask);
+cpumask_set_cpu(i, &vcpu_mask);
 }
 break;
 case SGI_TARGET_SELF:
@@ -355,8 +354,8 @@ int vgic_to_sgi(struct vcpu *v, register_t sgir, enum 
gic_sgi_mode irqmode, int
  * SGI_TARGET_SELF mode. So Force vcpu_mask to 0
  */
 perfc_incr(vgic_sgi_self);
-vcpu_mask = 0;
-set_bit(current->vcpu_id, &vcpu_mask);
+cpumask_clear(&vcpu_mask);
+cpumask_set_cpu(current->vcpu_id, &vcpu_mask);
 break;
 default:
 gprintk(XENLOG_WARNING,
@@ -365,12 +364,12 @@ int vgic_to_sgi(struct vcpu *v, register_t sgir, enum 
gic_sgi_mode irqmode, int
 return 0;
 }
 
-for_each_set_bit( vcpuid, &vcpu_mask, d->max_vcpus )
+for_each_cpu( vcpuid, &vcpu_mask )
 {
 if ( d->vcpu[vcpuid] != NULL && !is_vcpu_online(d->vcpu[vcpuid]) )
 {
 gprintk(XENLOG_WARNING, "VGIC: write r=%"PRIregister" \
-vcpu_mask=%lx, wrong CPUTargetList\n", sgir, vcpu_mask);
+, wrong CPUTargetList\n", sgir);
 continue;
 }
 vgic_vcpu_inject_irq(d->vcpu[vcpuid], v

[Xen-devel] [PATCH V4 8/8] xen/arm: make domain_max_vcpus return value according to vGIC version

[Chen Baozi]

From: Chen Baozi 

When a guest uses vGICv2, the maximum number of vCPU it can support
should not be as many as MAX_VIRT_CPUS, which is 128 at the moment.
So the domain_max_vcpus should return the value according to the vGIC
version the domain uses.

We didn't keep it as the old static inline form because it will break
compilation when access the member of struct domain:

In file included from xen/include/xen/domain.h:6:0,
 from xen/include/xen/sched.h:10,
 from arm64/asm-offsets.c:10:
xen/include/asm/domain.h: In function ‘domain_max_vcpus’:
xen/include/asm/domain.h:266:10: error: dereferencing pointer to incomplete type
 if (d->arch.vgic.version == GIC_V2)
  ^

Signed-off-by: Chen Baozi 
---
 xen/arch/arm/domain.c| 6 ++
 xen/include/asm-arm/domain.h | 5 +
 xen/include/asm-arm/gic.h| 3 +++
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 0cf147c..78b77b1 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -890,6 +890,12 @@ void vcpu_block_unless_event_pending(struct vcpu *v)
 vcpu_unblock(current);
 }
 
+unsigned int domain_max_vcpus(const struct domain *d)
+{
+return ((d->arch.vgic.version == GIC_V2) ?
+GICV2_MAX_CPUS : GICV3_MAX_CPUS);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 603a20b..c4cb15d 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -261,10 +261,7 @@ struct arch_vcpu
 void vcpu_show_execution_state(struct vcpu *);
 void vcpu_show_registers(const struct vcpu *);
 
-static inline unsigned int domain_max_vcpus(const struct domain *d)
-{
-return MAX_VIRT_CPUS;
-}
+unsigned int domain_max_vcpus(const struct domain *);
 
 /*
  * Due to the restriction of GICv3, the number of vCPUs in AFF0 is
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 463fffb..a7bc0d1 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -18,6 +18,9 @@
 #ifndef __ASM_ARM_GIC_H__
 #define __ASM_ARM_GIC_H__
 
+#define GICV2_MAX_CPUS  8
+#define GICV3_MAX_CPUS  128
+
 #define NR_GIC_LOCAL_IRQS  NR_LOCAL_IRQS
 #define NR_GIC_SGI 16
 #define MAX_RDIST_COUNT4
-- 
2.1.4


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

[Xen-devel] [PATCH V4 2/8] xen/arm: Add functions of mapping between vCPUID and virtual affinity

[Chen Baozi]

From: Chen Baozi 

GICv3 restricts that the maximum number of CPUs in affinity 0 (one
cluster) is 16. That is to say the upper 4 bits of affinity 0 is unused.
Current implementation considers that AFF0 is equal to vCPUID, which
makes all vCPUs in one cluster, limiting its number to 16. If we would
like to support more than 16 number of vCPU in one guest, we need to
make use of AFF1. Considering the unused upper 4 bits, we need to create
a pair of functions mapping the vCPUID and virtual affinity.

Signed-off-by: Chen Baozi 
---
 xen/include/asm-arm/domain.h | 36 
 1 file changed, 36 insertions(+)

diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 75b17af..603a20b 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -266,6 +266,42 @@ static inline unsigned int domain_max_vcpus(const struct 
domain *d)
 return MAX_VIRT_CPUS;
 }
 
+/*
+ * Due to the restriction of GICv3, the number of vCPUs in AFF0 is
+ * limited to 16, thus only the first 4 bits of AFF0 are legal. We will
+ * use the first 2 affinity levels here, expanding the number of vCPU up
+ * to 4096 (16*256), which is more than 128 PEs that GIC-500 supports.
+ *
+ * Since we don't save information of vCPU's topology (affinity) in
+ * vMPIDR at the moment, we map the vcpuid to the vMPIDR linearly.
+ *
+ * XXX: We may have multi-threading or virtual cluster information in
+ * the future.
+ */
+static inline unsigned int vaffinity_to_vcpuid(register_t vaff)
+{
+unsigned int vcpuid;
+
+vaff &= MPIDR_HWID_MASK;
+
+vcpuid = MPIDR_AFFINITY_LEVEL(vaff, 0);
+vcpuid |= MPIDR_AFFINITY_LEVEL(vaff, 1) << 4;
+
+return vcpuid;
+}
+
+static inline register_t vcpuid_to_vaffinity(unsigned int vcpuid)
+{
+register_t vaff;
+
+BUILD_BUG_ON(!(MAX_VIRT_CPUS < ((1 << 12;
+
+vaff = (vcpuid & 0x0f) << MPIDR_LEVEL_SHIFT(0);
+vaff |= ((vcpuid >> 4) & MPIDR_LEVEL_MASK) << MPIDR_LEVEL_SHIFT(1);
+
+return vaff;
+}
+
 #endif /* __ASM_DOMAIN_H__ */
 
 /*
-- 
2.1.4


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

[Xen-devel] [PATCH V4 1/8] xen/arm: gic-v3: Increase the size of GICR in address space for guest

[Chen Baozi]

From: Chen Baozi 

Currently it only supports up to 8 vCPUs. Increase the region to hold
up to 128 vCPUs, which is the maximum number that GIC-500 supports.

Signed-off-by: Chen Baozi 
Reviewed-by: Julien Grall 
---
 xen/include/public/arch-arm.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index c029e0f..ec0c261 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -388,8 +388,8 @@ struct xen_arch_domainconfig {
 #define GUEST_GICV3_RDIST_STRIDE   0x2ULL
 #define GUEST_GICV3_RDIST_REGIONS  1
 
-#define GUEST_GICV3_GICR0_BASE 0x0302ULL/* vCPU0 - vCPU7 */
-#define GUEST_GICV3_GICR0_SIZE 0x0010ULL
+#define GUEST_GICV3_GICR0_BASE 0x0302ULL/* vCPU0 - vCPU127 */
+#define GUEST_GICV3_GICR0_SIZE 0x0100ULL
 
 /*
  * 16MB == 4096 pages reserved for guest to use as a region to map its
-- 
2.1.4


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

[Xen-devel] [PATCH V4 0/8] Support more than 8 vcpus on arm64 with GICv3

[Chen Baozi]

From: Chen Baozi 

Currently the number of vcpus on arm64 with GICv3 is limited up to 8 due
to the fixed size of redistributor mmio region. Increasing the size
makes the number expand to 16 because of AFF0 restriction on GICv3.
To create a guest up to 128 vCPUs, which is the maxium number that GIC-500
can support, this patchset uses the AFF1 information to create a mapping
relation between vCPUID and vMPIDR and deals with the related issues.

These patches are written based upon Julien's "GICv2 on GICv3" series
and the IROUTER emulation cleanup patch.

Changes from V3:
* Drop the wrong patch that altering domain_max_vcpus to a macro.
* Change the domain_max_vcpus to return value accodring to the version
  of the vGIC in used.
Changes from V2:
* Reorder the patch which increases MAX_VIRT_CPUS to the last to make
  this series bisectable.
* Drop the dynamic re-distributor region allocation patch in tools.
* Use cpumask_t type instead of unsigned long in vgic_to_sgi and do the
  translation from GICD_SGIR to vcpu_mask in both vGICv2 and vGICv3.
* Make domain_max_vcpus be alias of max_vcpus in struct domain
Changes from V1:
* Use the way that expanding the GICR address space to support up to 128
  redistributor in guest memory layout rather than use the dynamic
  allocation.
* Add support to include AFF1 information in vMPIDR/logical CPUID.


Chen Baozi (8):
  xen/arm: gic-v3: Increase the size of GICR in address space for guest
  xen/arm: Add functions of mapping between vCPUID and virtual affinity
  xen/arm: Use the new functions for vCPUID/vaffinity transformation
  xen/arm: Use cpumask_t type for vcpu_mask in vgic_to_sgi
  tools/libxl: Set 'reg' of cpu node equal to MPIDR affinity for domU
  xen/arm: Set 'reg' of cpu node for dom0 to match MPIDR's affinity
  xen/arm64: increase MAX_VIRT_CPUS to 128 on arm64
  xen/arm: make domain_max_vcpus return value according to vGIC version

 tools/libxl/libxl_arm.c   | 14 --
 xen/arch/arm/domain.c | 12 +++-
 xen/arch/arm/domain_build.c   | 14 +++---
 xen/arch/arm/vgic-v2.c| 16 ++--
 xen/arch/arm/vgic-v3.c| 22 +-
 xen/arch/arm/vgic.c   | 15 +++
 xen/arch/arm/vpsci.c  |  2 +-
 xen/include/asm-arm/config.h  |  4 
 xen/include/asm-arm/domain.h  | 37 +++--
 xen/include/asm-arm/gic.h |  3 +++
 xen/include/asm-arm/vgic.h|  2 +-
 xen/include/public/arch-arm.h |  4 ++--
 12 files changed, 114 insertions(+), 31 deletions(-)

-- 
2.1.4


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

[Xen-devel] [PATCH V4 7/8] xen/arm64: increase MAX_VIRT_CPUS to 128 on arm64

[Chen Baozi]

From: Chen Baozi 

GIC-500 supports up to 128 cores in a single SoC. Increase MAX_VIRT_CPUS
to 128 on arm64.

Signed-off-by: Chen Baozi 
---
 xen/arch/arm/vgic-v3.c   | 1 -
 xen/include/asm-arm/config.h | 4 
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 0da031c..be5fff1 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -889,7 +889,6 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, 
mmio_info_t *info)
 rank = vgic_rank_offset(v, 64, gicd_reg - GICD_IROUTER,
 DABT_DOUBLE_WORD);
 if ( rank == NULL ) goto write_ignore;
-BUG_ON(v->domain->max_vcpus > 8);
 new_irouter = *r;
 vgic_lock_rank(v, rank, flags);
 
diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index 3b23e05..817c216 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -47,7 +47,11 @@
 #define NR_CPUS 128
 #endif
 
+#ifdef CONFIG_ARM_64
+#define MAX_VIRT_CPUS 128
+#else
 #define MAX_VIRT_CPUS 8
+#endif
 
 #define asmlinkage /* Nothing needed */
 
-- 
2.1.4


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

[Xen-devel] [PATCH V4 3/8] xen/arm: Use the new functions for vCPUID/vaffinity transformation

[Chen Baozi]

From: Chen Baozi 

There are 3 places to change:

* Initialise vMPIDR value in vcpu_initialise()
* Find the vCPU from vMPIDR affinity information when accessing GICD
  registers in vGIC
* Find the vCPU from vMPIDR affinity information when booting with vPSCI
  in vGIC

Signed-off-by: Chen Baozi 
---
 xen/arch/arm/domain.c  | 6 +-
 xen/arch/arm/vgic-v3.c | 2 +-
 xen/arch/arm/vpsci.c   | 2 +-
 3 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 2bde26e..0cf147c 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -501,11 +501,7 @@ int vcpu_initialise(struct vcpu *v)
 
 v->arch.sctlr = SCTLR_GUEST_INIT;
 
-/*
- * By default exposes an SMP system with AFF0 set to the VCPU ID
- * TODO: Handle multi-threading processor and cluster
- */
-v->arch.vmpidr = MPIDR_SMP | (v->vcpu_id << MPIDR_AFF0_SHIFT);
+v->arch.vmpidr = MPIDR_SMP | vcpuid_to_vaffinity(v->vcpu_id);
 
 v->arch.actlr = READ_SYSREG32(ACTLR_EL1);
 
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 540f85f..ef9a71a 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -61,7 +61,7 @@ static struct vcpu *vgic_v3_irouter_to_vcpu(struct domain *d, 
uint64_t irouter)
 if ( irouter & GICD_IROUTER_SPI_MODE_ANY )
 return d->vcpu[0];
 
-vcpu_id = irouter & MPIDR_AFF0_MASK;
+vcpu_id = vaffinity_to_vcpuid(irouter);
 if ( vcpu_id >= d->max_vcpus )
 return NULL;
 
diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
index 5d899be..1c1e7de 100644
--- a/xen/arch/arm/vpsci.c
+++ b/xen/arch/arm/vpsci.c
@@ -33,7 +33,7 @@ static int do_common_cpu_on(register_t target_cpu, register_t 
entry_point,
 register_t vcpuid;
 
 if( ver == XEN_PSCI_V_0_2 )
-vcpuid = (target_cpu & MPIDR_HWID_MASK);
+vcpuid = vaffinity_to_vcpuid(target_cpu);
 else
 vcpuid = target_cpu;
 
-- 
2.1.4


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

[Xen-devel] [PATCH V4 5/8] tools/libxl: Set 'reg' of cpu node equal to MPIDR affinity for domU

[Chen Baozi]

From: Chen Baozi 

According to ARM CPUs bindings, the reg field should match the MPIDR's
affinity bits. We will use AFF0 and AFF1 when constructing the reg value
of the guest at the moment, for it is enough for the current max vcpu
number.

Signed-off-by: Chen Baozi 
---
 tools/libxl/libxl_arm.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index c5088c4..8aa4815 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -272,6 +272,7 @@ static int make_cpus_node(libxl__gc *gc, void *fdt, int 
nr_cpus,
   const struct arch_info *ainfo)
 {
 int res, i;
+uint64_t mpidr_aff;
 
 res = fdt_begin_node(fdt, "cpus");
 if (res) return res;
@@ -283,7 +284,16 @@ static int make_cpus_node(libxl__gc *gc, void *fdt, int 
nr_cpus,
 if (res) return res;
 
 for (i = 0; i < nr_cpus; i++) {
-const char *name = GCSPRINTF("cpu@%d", i);
+const char *name;
+
+/*
+ * According to ARM CPUs bindings, the reg field should match
+ * the MPIDR's affinity bits. We will use AFF0 and AFF1 when
+ * constructing the reg value of the guest at the moment, for it
+ * is enough for the current max vcpu number.
+ */
+mpidr_aff = (uint64_t)((i & 0x0f) | (((i >> 4) & 0xff) << 8));
+name = GCSPRINTF("cpu@%lx", mpidr_aff);
 
 res = fdt_begin_node(fdt, name);
 if (res) return res;
@@ -297,7 +307,7 @@ static int make_cpus_node(libxl__gc *gc, void *fdt, int 
nr_cpus,
 res = fdt_property_string(fdt, "enable-method", "psci");
 if (res) return res;
 
-res = fdt_property_regs(gc, fdt, 1, 0, 1, (uint64_t)i);
+res = fdt_property_regs(gc, fdt, 1, 0, 1, mpidr_aff);
 if (res) return res;
 
 res = fdt_end_node(fdt);
-- 
2.1.4


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

[Xen-devel] [PATCH V4 6/8] xen/arm: Set 'reg' of cpu node for dom0 to match MPIDR's affinity

[Chen Baozi]

From: Chen Baozi 

According to ARM CPUs bindings, the reg field should match the MPIDR's
affinity bits. We will use AFF0 and AFF1 when constructing the reg value
of the guest at the moment, for it is enough for the current max vcpu
number.

Signed-off-by: Chen Baozi 
---
 xen/arch/arm/domain_build.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index a156de9..5591d82 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -712,6 +712,7 @@ static int make_cpus_node(const struct domain *d, void *fdt,
 char buf[15];
 u32 clock_frequency;
 bool_t clock_valid;
+uint32_t mpidr_aff;
 
 DPRINT("Create cpus node\n");
 
@@ -761,9 +762,16 @@ static int make_cpus_node(const struct domain *d, void 
*fdt,
 
 for ( cpu = 0; cpu < d->max_vcpus; cpu++ )
 {
-DPRINT("Create cpu@%u node\n", cpu);
+/*
+ * According to ARM CPUs bindings, the reg field should match
+ * the MPIDR's affinity bits. We will use AFF0 and AFF1 when
+ * constructing the reg value of the guest at the moment, for it
+ * is enough for the current max vcpu number.
+ */
+mpidr_aff = vcpuid_to_vaffinity(cpu);
+DPRINT("Create cpu@%x node\n", mpidr_aff);
 
-snprintf(buf, sizeof(buf), "cpu@%u", cpu);
+snprintf(buf, sizeof(buf), "cpu@%x", mpidr_aff);
 res = fdt_begin_node(fdt, buf);
 if ( res )
 return res;
@@ -776,7 +784,7 @@ static int make_cpus_node(const struct domain *d, void *fdt,
 if ( res )
 return res;
 
-res = fdt_property_cell(fdt, "reg", cpu);
+res = fdt_property_cell(fdt, "reg", mpidr_aff);
 if ( res )
 return res;
 
-- 
2.1.4


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

Re: [Xen-devel] [xen-unstable test] 56759: regressions - FAIL

[Jan Beulich]

>>> On 28.05.15 at 11:26,  wrote:
> On Thu, 2015-05-28 at 09:50 +0100, Jan Beulich wrote:
>> >>> On 27.05.15 at 18:04,  wrote:
>> > On Tue, 2015-05-26 at 14:29 +0100, Ian Campbell wrote:
>> >> I've now managed to reproduce using the arndale on my desk.
>> > 
>> > ... and now I've confirmed that reverting the spin lock change causes
>> > the issue to not happen any more.
>> 
>> Considering that this issue has prevented a push for almost
>> two weeks, I think we ought to consider reverting the two
>> offending commits until the problem got sorted out.
> 
> I think that would probably be wise. I'll try and figure out exactly
> what is going on and propose some patches ASAP.

Now done and pushed.

Jan


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

Re: [Xen-devel] [libvirt] [PATCH 3/4] libxl: support SPICE graphics for HVM domains

[Michal Privoznik]

On 09.05.2015 00:31, Jim Fehlig wrote:
> Signed-off-by: Jim Fehlig 
> ---
>  src/libxl/libxl_conf.c | 72 
> +-
>  1 file changed, 66 insertions(+), 6 deletions(-)
> 
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 8552c77..5bb0425 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -1337,22 +1337,82 @@ libxlMakeVfbList(virPortAllocatorPtr graphicsports,
>  
>  /*
>   * Populate vfb info in libxl_domain_build_info struct for HVM domains.
> - * Use first libxl_device_vfb device in libxl_domain_config->vfbs.
> - * Prior to calling this function, libxlMakeVfbList must be called to
> - * populate libxl_domain_config->vfbs.
> + * Prefer SPICE, otherwise use first libxl_device_vfb device in
> + * libxl_domain_config->vfbs. Prior to calling this function,
> + * libxlMakeVfbList must be called to populate libxl_domain_config->vfbs.
>   */
>  static int
> -libxlMakeBuildInfoVfb(virDomainDefPtr def, libxl_domain_config *d_config)
> +libxlMakeBuildInfoVfb(virPortAllocatorPtr graphicsports,
> +  virDomainDefPtr def,
> +  libxl_domain_config *d_config)
>  {
>  libxl_domain_build_info *b_info = &d_config->b_info;
>  libxl_device_vfb x_vfb;
> +size_t i;
>  
>  if (def->os.type != VIR_DOMAIN_OSTYPE_HVM)
>  return 0;
>  
> -if (d_config->num_vfbs == 0)
> +if (def->ngraphics == 0)
>  return 0;
>  
> +for (i = 0; i < def->ngraphics; i++) {
> +virDomainGraphicsDefPtr l_vfb = def->graphics[0];

This seems really awkward to me. So you're using for() loop just to
check if the first graphics card (assuming there can't be more than one
anyway) is SPICE. If not, you could use 'continue' to continue with VNC.
I think this obfucates the code. Just move this into a separate function
and call it from here.

> +unsigned short port;
> +const char *listenAddr;
> +
> +if (l_vfb->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE)
> +continue;
> +
> +libxl_defbool_set(&b_info->u.hvm.spice.enable, true);
> +
> +if (l_vfb->data.spice.autoport) {
> +if (virPortAllocatorAcquire(graphicsports, &port) < 0)
> +return -1;
> +l_vfb->data.spice.port = port;
> +}
> +b_info->u.hvm.spice.port = l_vfb->data.spice.port;
> +
> +listenAddr = virDomainGraphicsListenGetAddress(l_vfb, 0);
> +if (VIR_STRDUP(b_info->u.hvm.spice.host, listenAddr) < 0)
> +return -1;
> +
> +if (VIR_STRDUP(b_info->u.hvm.keymap, l_vfb->data.spice.keymap) < 0)
> +return -1;
> +
> +if (l_vfb->data.spice.auth.passwd) {
> +if (VIR_STRDUP(b_info->u.hvm.spice.passwd,
> +   l_vfb->data.spice.auth.passwd) < 0)
> +return -1;
> +libxl_defbool_set(&b_info->u.hvm.spice.disable_ticketing, false);
> +} else {
> +libxl_defbool_set(&b_info->u.hvm.spice.disable_ticketing, true);
> +}
> +
> +switch (l_vfb->data.spice.mousemode) {
> +/* client mouse mode is default in xl.cfg */
> +case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_DEFAULT:
> +case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_CLIENT:
> +libxl_defbool_set(&b_info->u.hvm.spice.agent_mouse, true);
> +break;
> +case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_SERVER:
> +libxl_defbool_set(&b_info->u.hvm.spice.agent_mouse, false);
> +break;
> +}
> +
> +#ifdef LIBXL_HAVE_SPICE_VDAGENT
> +if (l_vfb->data.spice.copypaste == VIR_TRISTATE_BOOL_YES) {
> +libxl_defbool_set(&b_info->u.hvm.spice.vdagent, true);
> +libxl_defbool_set(&b_info->u.hvm.spice.clipboard_sharing, true);
> +} else {
> +libxl_defbool_set(&b_info->u.hvm.spice.vdagent, false);
> +libxl_defbool_set(&b_info->u.hvm.spice.clipboard_sharing, false);
> +}
> +#endif
> +
> +return 0;
> +}
> +
>  x_vfb = d_config->vfbs[0];
>  
>  if (libxl_defbool_val(x_vfb.vnc.enable)) {
> @@ -1778,7 +1838,7 @@ libxlBuildDomainConfig(virPortAllocatorPtr 
> graphicsports,
>  if (libxlMakeVfbList(graphicsports, def, d_config) < 0)
>  return -1;
>  
> -if (libxlMakeBuildInfoVfb(def, d_config) < 0)
> +if (libxlMakeBuildInfoVfb(graphicsports, def, d_config) < 0)
>  return -1;
>  
>  if (libxlMakePCIList(def, d_config) < 0)
> 

Otherwise looking good.

Michal

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

Re: [Xen-devel] [PATCH v7 04/10] xen: Introduce XEN_DOMCTL_soft_reset

[Tim Deegan]

Hi,

At 17:25 +0200 on 27 May (1432747540), Vitaly Kuznetsov wrote:
> New operation reassigns all memory pages from source domain to the destination
> domain mapping them at exactly the same GFNs. Pages mapped more than once 
> (e.g.
> grants) are being copied.
> 
> Signed-off-by: Vitaly Kuznetsov 
> ---
> Changes in v7:
> - is_soft_reset flag added to struct domain to preserve destination domain's
>   paused state across possible hypercall preemption.
> [Jan Beulich]
> - XENMEM_soft_reset -> XEN_DOMCTL_soft_reset
> - domain_soft_reset() returns int
> - no locking for is_dying as it is now proteced by domctl_lock
> - print a warning on !mfn_valid case
> - check PGC_allocated for pages
> - no print on assign_pages failure (it prints error messages on both its 
> failure paths)
> - don't re-read page->count_info, use copy_page flag
> - minor stucturing change
> - pause both source and destination domain while processing the hypercall
> - remove nr_transferred from public interface
> [Tim Deegan]
> - use get_gfn_type_access() in PoD case (x86-only)
> ---
>  xen/common/domain.c | 186 
> 
>  xen/common/domctl.c |  72 +
>  xen/include/public/domctl.h |  28 +++
>  xen/include/xen/sched.h |   6 ++
>  4 files changed, 292 insertions(+)
> 
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 7825c56..824f325 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -1006,6 +1006,192 @@ int domain_unpause_by_systemcontroller(struct domain 
> *d)
>  return 0;
>  }
>  
> +int domain_soft_reset(struct domain *source_d, struct domain *dest_d,
> +  xen_pfn_t *gmfn_start)
> +{
> +int rc = 0;
> +unsigned long mfn, mfn_new, gmfn, last_gmfn, count;
> +unsigned int order;
> +p2m_type_t p2mt;
> +struct page_info *page, *new_page, *next_page;
> +int drop_dom_ref, copy_page;
> +
> +last_gmfn = domain_get_maximum_gpfn(source_d);
> +gmfn = *gmfn_start;
> +while ( gmfn <= last_gmfn )
> +{
> +page = get_page_from_gfn(source_d, gmfn, &p2mt, 0);

In order to be safe against p2m changes, you should use
get_gfn_type_access() _here_, and put_gfn() when you're finished with the
gfn.  You'll also need to get_page() the returned MFN, of course, to
make sure that it isn't freed before you reassign it.

> +if ( unlikely(page == NULL) )
> +{
> +#ifdef CONFIG_X86
> +struct p2m_domain *p2m = p2m_get_hostp2m(source_d);
> +p2m_access_t p2ma;
> +mfn_t mfn_p2m;
> +
> +order = 0;
> +mfn_p2m = get_gfn_type_access(p2m, gmfn,
> +  &p2mt, &p2ma, 0, &order);
> +if ( p2m_is_pod(p2mt) )
> +{
> +rc = guest_physmap_mark_populate_on_demand(dest_d, gmfn,
> +   order);
> +if ( unlikely(rc) )
> +{
> +printk(XENLOG_G_ERR "Failed to mark Dom%d's GFN %lx"
> +   " (order: %d) as PoD\n", source_d->domain_id,
> +   gmfn, order);
> +goto fail;
> +}
> +}
> +put_gfn(source_d, gmfn);
> +gmfn += 1ul << order;
> +#else
> +gmfn++;
> +#endif
> +goto preempt_check;
> +}
> +
> +mfn = page_to_mfn(page);
> +if ( unlikely(!mfn_valid(mfn)) )
> +{
> +printk(XENLOG_G_WARNING "Dom%d's GFN %lx points to invalid 
> MFN\n",
> +   source_d->domain_id, gmfn);
> +put_page(page);
> +gmfn++;
> +goto preempt_check;
> +}
> +
> +next_page = page;
> +count = 0;
> +copy_page = 0;
> +while ( next_page && !is_xen_heap_page(next_page) &&
> +page_to_mfn(next_page) == mfn + count )

What's the purpose of this second loop?  It doesn't seem to be doing
anything that the outer loop couldn't.

Cheers,

Tim.

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

[Xen-devel] [rumpuserxen test] 57443: regressions - FAIL

[osstest service user]

flight 57443 rumpuserxen real [real]
http://logs.test-lab.xenproject.org/osstest/logs/57443/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-rumpuserxen   5 rumpuserxen-build fail REGR. vs. 33866
 build-i386-rumpuserxen5 rumpuserxen-build fail REGR. vs. 33866

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)   blocked n/a
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a

version targeted for testing:
 rumpuserxen  3b91e44996ea6ae1276bce1cc44f38701c53ee6f
baseline version:
 rumpuserxen  30d72f3fc5e35cd53afd82c8179cc0e0b11146ad


People who touched revisions under test:
  Antti Kantee 
  Ian Jackson 
  Martin Lucina 
  Wei Liu 


jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 build-amd64-rumpuserxen  fail
 build-i386-rumpuserxen   fail
 test-amd64-amd64-rumpuserxen-amd64   blocked 
 test-amd64-i386-rumpuserxen-i386 blocked 



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

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 535 lines long.)

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

Re: [Xen-devel] [PATCH v2 4/9] x86/intel_pstate: add new policy fields and a new driver interface

[Jan Beulich]

>>> On 28.05.15 at 11:47,  wrote:
> On 26/05/2015 21:00, Jan Beulich wrote
>> >>> On 13.05.16 at 09:50,  wrote:
>> > --- a/xen/include/acpi/cpufreq/cpufreq.h
>> > +++ b/xen/include/acpi/cpufreq/cpufreq.h
>> > @@ -52,6 +52,10 @@ struct cpufreq_policy {
>> >  unsigned intmax;/* in kHz */
>> >  unsigned intcur;/* in kHz, only needed if cpufreq
>> >   * governors are used */
>> > +int min_perf_pct; /* min performance in percentage */
>> > +int max_perf_pct; /* max performance in percentage */
>> > +int turbo_pct;
>> 
>> Can any of these legitimately be negative? If not, they should be of unsigned
>> int type.
> 
> I think we have to keep using "int". Though the expected pct value range is 
> 0-100, it is possible that people may input a negative value. For example, if 
> the input value is "-1": 
> 1) using int, the output value after clamp_t(0,100) will be 0;
> 2) using unsigned int, the output value after clamp_t(0,100) will be 100.
> I think case 1) is what we need.  

There are no "people" involved here. If you think negative values
make sense to be accepted by the xenpm tool, deal with them there.
Already at the hypercall interface only sane values should be passed
(and insane ones, like percentage values outside the [0,100] range,
probably be rejected).

>> > @@ -87,6 +91,12 @@ struct cpufreq_freqs {
>> >   *  CPUFREQ GOVERNORS*
>> >
>> >
>> **
>> ***/
>> >
>> > +/* the four internal governors used in intel_pstate */
>> > +#define CPUFREQ_POLICY_POWERSAVE(1)
>> > +#define CPUFREQ_POLICY_PERFORMANCE  (2)
>> > +#define CPUFREQ_POLICY_USERSPACE(3)
>> > +#define CPUFREQ_POLICY_ONDEMAND (4)
>> 
>> Why are they being added in this header and at this point in the sequence of
>> patches? If they're local to intel_pstate, they should go there. If they're
>> needed by multiple files, they should be added when needed, so that one
>> can easily judge whether their addition is indeed necessary.
> 
> Will change to add the 4 macros in the "main body of intel_pstate driver" 
> patch (6/9). 
> Can we keep them in this file? These are cpufreq related things.

Perhaps - what I do mind is the combination of the when and where.
Fixing the when may eliminate the where complaint.

Jan


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

Re: [Xen-devel] [PATCH] [DOCS/RFC]: Xen: Clarification to terms used in hypervisor memory management

[Tim Deegan]

At 17:49 +0100 on 27 May (1432748951), Andrew Cooper wrote:
> RFC: All changes here are subject to my current understanding, which might be 
> wrong
> 
> Memory management is hard[citation needed].  Furthermore, it isn't helped by
> the inconsistent use of terms through the code.
> 
> Describe the currently-used terms in more practical terms.
> 
> Signed-off-by: Andrew Cooper 

Acked-by: Tim Deegan 

> The term 'pfn' is most little awkward, as it currently has two uses in
> different areas of the code.  I would argue that the caller of the pagetable
> code ought to know exactly if it dealing with an mfn or a gfn in the
> pagetables, at which point pfn from the old point of view becomes redundant.

Yes, I would hope that we can go that direction over time.

Cheers,

Tim.

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

Re: [Xen-devel] 4.5.1-rc1 has been tagged

[Jan Beulich]

>>> On 28.05.15 at 11:38,  wrote:
> I had one total crash/reboot of Xen with pvops domU kernel 3.18.13. 
> Somehow it looked like that domU was able to crash the whole hypervisor. 
> Perhaps somebody has time to spend more time on investigating.

Seems unlikely without you giving any details.

Nevertheless - thanks for the testing!

Jan


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

Re: [Xen-devel] [PATCH v2 4/9] x86/intel_pstate: add new policy fields and a new driver interface

[Wang, Wei W]

On 26/05/2015 21:00, Jan Beulich wrote
> >>> On 13.05.16 at 09:50,  wrote:
> > --- a/xen/drivers/cpufreq/utility.c
> > +++ b/xen/drivers/cpufreq/utility.c
> > @@ -456,6 +456,11 @@ int __cpufreq_set_policy(struct cpufreq_policy
> > *data,
> >
> >  data->min = policy->min;
> >  data->max = policy->max;
> > +data->min_perf_pct = policy->min_perf_pct;
> > +data->max_perf_pct = policy->max_perf_pct;
> 
> Shouldn't you do similar sanity checks on them as are being done on
> ->min and ->max? (Admittedly they look a little strange, but I'm not
> asking you to copy what is there without sanity checking the original).
> 
> > +if (cpufreq_driver->setpolicy)
> 
> Also I wonder whether the additions shouldn't move into this if()'s body.

Will change it to the if()'s body:
if (cpufreq_driver->setpolicy) {
data->min_perf_pct = policy->min_perf_pct;
data->max_perf_pct = policy->max_perf_pct;

}


> 
> > --- a/xen/include/acpi/cpufreq/cpufreq.h
> > +++ b/xen/include/acpi/cpufreq/cpufreq.h
> > @@ -52,6 +52,10 @@ struct cpufreq_policy {
> >  unsigned intmax;/* in kHz */
> >  unsigned intcur;/* in kHz, only needed if cpufreq
> >   * governors are used */
> > +int min_perf_pct; /* min performance in percentage */
> > +int max_perf_pct; /* max performance in percentage */
> > +int turbo_pct;
> 
> Can any of these legitimately be negative? If not, they should be of unsigned
> int type.

I think we have to keep using "int". Though the expected pct value range is 
0-100, it is possible that people may input a negative value. For example, if 
the input value is "-1": 
1) using int, the output value after clamp_t(0,100) will be 0;
2) using unsigned int, the output value after clamp_t(0,100) will be 100.
I think case 1) is what we need.  


> 
> > @@ -87,6 +91,12 @@ struct cpufreq_freqs {
> >   *  CPUFREQ GOVERNORS*
> >
> >
> **
> ***/
> >
> > +/* the four internal governors used in intel_pstate */
> > +#define CPUFREQ_POLICY_POWERSAVE(1)
> > +#define CPUFREQ_POLICY_PERFORMANCE  (2)
> > +#define CPUFREQ_POLICY_USERSPACE(3)
> > +#define CPUFREQ_POLICY_ONDEMAND (4)
> 
> Why are they being added in this header and at this point in the sequence of
> patches? If they're local to intel_pstate, they should go there. If they're
> needed by multiple files, they should be added when needed, so that one
> can easily judge whether their addition is indeed necessary.

Will change to add the 4 macros in the "main body of intel_pstate driver" patch 
(6/9). 
Can we keep them in this file? These are cpufreq related things.

Best,
Wei


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

Re: [Xen-devel] [PATCH v2 3/9] x86/cpu_hotplug: add the unregister_cpu_notifier function to support CPU hotplug

[Jan Beulich]

>>> On 28.05.15 at 11:26,  wrote:
> On 26/05/2015 20:51, Jan Beulich wrote
>> >>> On 13.05.16 at 09:50,  wrote:
>> > The unregister notifier function is needed to support cpu hotplug.
>> 
>> Without loadable module support I have some difficulty seeing why this
>> should be needed.
>> 
> 
> Ok. Then, should we remove the entire cpufreq_unregister_driver() function 
> (the unregister notifier function is invoked inside)?  Because there is also 
> no chance to unregister the driver.

Sure - it looks to be unused at present anyway.

Jan


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

Re: [Xen-devel] 4.5.1-rc1 has been tagged

[Andreas Kinzler]

I am currently validating 4.5.1-rc1 as a stable platform for production 
environments. I perform a series of tests which stress the IO subsystems 
(net+disk) to the max. For block IO I reach more than 1.5 gigabytes/sec. 
The tests also hash (SHA1) all IO and verify it against known values so 
I can be sure that traffic is not "write only" but really transferred 
correctly bit-by-bit. Tests are also highly multithreaded, so there 
should be some good coverage.


So far these tests were without any problem.

I had one total crash/reboot of Xen with pvops domU kernel 3.18.13. 
Somehow it looked like that domU was able to crash the whole hypervisor. 
Perhaps somebody has time to spend more time on investigating.


Regards Andreas

On 18.05.2015 16:27, Jan Beulich wrote:

All,
aiming at a release with presumably (i.e. as usual) one more RC,
please test!
Thanks, Jan




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

Re: [Xen-devel] [xen-unstable test] 56759: regressions - FAIL

[Ian Campbell]

On Thu, 2015-05-28 at 09:50 +0100, Jan Beulich wrote:
> >>> On 27.05.15 at 18:04,  wrote:
> > On Tue, 2015-05-26 at 14:29 +0100, Ian Campbell wrote:
> >> On Wed, 2015-05-20 at 10:56 +0100, Ian Campbell wrote:
> >> > On Wed, 2015-05-20 at 09:34 +, osstest service user wrote:
> >> > > flight 56759 xen-unstable real [real]
> >> > > http://logs.test-lab.xenproject.org/osstest/logs/56759/ 
> >> > > 
> >> > > Regressions :-(
> >> > > 
> >> > > Tests which did not succeed and are blocking,
> >> > > including tests which could not be run:
> >> > >  test-armhf-armhf-xl-multivcpu 17 leak-check/check fail REGR. 
> >> > > vs. 56375
> >> > 
> >> > I'm pretty hard pressed to explain this from the set of commits
> >> > currently under test, but it has happened a few times now (e.g. 56700
> >> > 56576) so it does seem to be real.
> >> > 
> >> > 
> > http://logs.test-lab.xenproject.org/osstest/results/bisect.xen-unstable.test-ar
> >  
> > mhf-armhf-xl-multivcpu.leak-check--check.html
> >> > is working on it and is currently consider the set of changes from:
> >> > ianc@cosworth:xen.git$ git log --oneline 9ab42~1...45fcc4
> >> > 45fcc45 use ticket locks for spin locks
> >> > e13013d libxc/restore: add checkpointed flag to the restore context
> >> > ce44b40 libxc/restore: introduce setup() and cleanup() on restore
> >> > c5c5a04 libxc/restore: split read/handle qemu info
> >> > 9ab42c9 libxc/restore: introduce process_record()
> >> > 
> >> > where e13013d is current master which was pushed in by flight 56375.
> >> > 
> >> > I think it unlikely the libxl stuff is responible, given we don't
> >> > migrate on ARM, which would seem to point to the ticket locks...
> >> 
> >> I've now managed to reproduce using the arndale on my desk.
> > 
> > ... and now I've confirmed that reverting the spin lock change causes
> > the issue to not happen any more.
> 
> Considering that this issue has prevented a push for almost
> two weeks, I think we ought to consider reverting the two
> offending commits until the problem got sorted out.

I think that would probably be wise. I'll try and figure out exactly
what is going on and propose some patches ASAP.

Ian.



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

Re: [Xen-devel] [PATCH v2 3/9] x86/cpu_hotplug: add the unregister_cpu_notifier function to support CPU hotplug

[Wang, Wei W]

On 26/05/2015 20:51, Jan Beulich wrote
> >>> On 13.05.16 at 09:50,  wrote:
> > The unregister notifier function is needed to support cpu hotplug.
> 
> Without loadable module support I have some difficulty seeing why this
> should be needed.
> 

Ok. Then, should we remove the entire cpufreq_unregister_driver() function (the 
unregister notifier function is invoked inside)?  Because there is also no 
chance to unregister the driver.

Best,
Wei

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

Re: [Xen-devel] [PATCH V3 8/8] xen/arm: make domain_max_vcpus be alias of max_vcpus in struct domain

[Chen Baozi]

Hi Andrew,

On Thu, May 28, 2015 at 09:50:38AM +0100, Andrew Cooper wrote:
> On 28/05/15 08:44, Chen Baozi wrote:
> > From: Chen Baozi 
> >
> > Since the maximum vcpu information is already saved in the struct domain,
> > there is no need for domain_max_vpus to return the fixed value.
> >
> > Signed-off-by: Chen Baozi 
> > ---
> >  xen/include/asm-arm/domain.h | 5 +
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
> > index 603a20b..b4e38a2 100644
> > --- a/xen/include/asm-arm/domain.h
> > +++ b/xen/include/asm-arm/domain.h
> > @@ -261,10 +261,7 @@ struct arch_vcpu
> >  void vcpu_show_execution_state(struct vcpu *);
> >  void vcpu_show_registers(const struct vcpu *);
> >  
> > -static inline unsigned int domain_max_vcpus(const struct domain *d)
> > -{
> > -return MAX_VIRT_CPUS;
> > -}
> > +#define domain_max_vcpus(d) (d->max_vcpus)
> 
> First of all, don't go altering a properly typed static inline like this
> for a macro.  The former is better in all regards, save the number of
> lines it takes to express.
> 
> You appear to have missed the entire point of this function.
> d->max_vcpus is uninitialised at this point (although it will always
> have the value 0).

Ah, yes. I didn't noticed that it is called at the very beginning of
XEN_DOMCTL_max_vcpus, where d->max_vcpus is not initialized.

> 
> You have also missed the point of Juliens review, which is to say that
> an ARM64 build of Xen running a GICv2 domain must not claim to support
> 128 cpus.
> 

Due to my misunderstanding, I thought the d->max_vcpus would store the right
value which has already been ajusted according to vGIC's version.

I'll resend a V4 to fix it.

Thanks.

Baozi.

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